* [PATCH 1/3] media: ov5640: initialize mode data structs by name
@ 2018-04-20 9:44 Daniel Mack
2018-04-20 9:44 ` [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls Daniel Mack
2018-04-20 9:44 ` [PATCH 3/3] media: ov5640: add support for xclk frequency control Daniel Mack
0 siblings, 2 replies; 11+ messages in thread
From: Daniel Mack @ 2018-04-20 9:44 UTC (permalink / raw)
To: linux-media; +Cc: slongerbeam, mchehab, Daniel Mack
This patch initializes the members of struct ov5640_mode_info by name for
better readability. This makes later additions to this struct easier.
No functional change intended.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/media/i2c/ov5640.c | 207 +++++++++++++++++++++++++++++++++------------
1 file changed, 152 insertions(+), 55 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 852026baa2e7..96f1564abdf5 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -728,67 +728,164 @@ static const struct reg_value ov5640_setting_15fps_QSXGA_2592_1944[] = {
/* power-on sensor init reg table */
static const struct ov5640_mode_info ov5640_mode_init_data = {
- 0, SUBSAMPLING, 640, 480, ov5640_init_setting_30fps_VGA,
- ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
+ .id = 0,
+ .dn_mode = SUBSAMPLING,
+ .width = 640,
+ .height = 480,
+ .reg_data = ov5640_init_setting_30fps_VGA,
+ .reg_data_size = ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
};
static const struct ov5640_mode_info
ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
+{
+ {
+ .id = OV5640_MODE_QCIF_176_144,
+ .dn_mode = SUBSAMPLING,
+ .width = 176,
+ .height = 144,
+ .reg_data = ov5640_setting_15fps_QCIF_176_144,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144),
+ },
+ {
+ .id = OV5640_MODE_QVGA_320_240,
+ .dn_mode = SUBSAMPLING,
+ .width = 320,
+ .height = 240,
+ .reg_data = ov5640_setting_15fps_QVGA_320_240,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240),
+ },
{
- {OV5640_MODE_QCIF_176_144, SUBSAMPLING, 176, 144,
- ov5640_setting_15fps_QCIF_176_144,
- ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
- {OV5640_MODE_QVGA_320_240, SUBSAMPLING, 320, 240,
- ov5640_setting_15fps_QVGA_320_240,
- ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
- {OV5640_MODE_VGA_640_480, SUBSAMPLING, 640, 480,
- ov5640_setting_15fps_VGA_640_480,
- ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
- {OV5640_MODE_NTSC_720_480, SUBSAMPLING, 720, 480,
- ov5640_setting_15fps_NTSC_720_480,
- ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
- {OV5640_MODE_PAL_720_576, SUBSAMPLING, 720, 576,
- ov5640_setting_15fps_PAL_720_576,
- ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
- {OV5640_MODE_XGA_1024_768, SUBSAMPLING, 1024, 768,
- ov5640_setting_15fps_XGA_1024_768,
- ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
- {OV5640_MODE_720P_1280_720, SUBSAMPLING, 1280, 720,
- ov5640_setting_15fps_720P_1280_720,
- ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
- {OV5640_MODE_1080P_1920_1080, SCALING, 1920, 1080,
- ov5640_setting_15fps_1080P_1920_1080,
- ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
- {OV5640_MODE_QSXGA_2592_1944, SCALING, 2592, 1944,
- ov5640_setting_15fps_QSXGA_2592_1944,
- ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
- }, {
- {OV5640_MODE_QCIF_176_144, SUBSAMPLING, 176, 144,
- ov5640_setting_30fps_QCIF_176_144,
- ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
- {OV5640_MODE_QVGA_320_240, SUBSAMPLING, 320, 240,
- ov5640_setting_30fps_QVGA_320_240,
- ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
- {OV5640_MODE_VGA_640_480, SUBSAMPLING, 640, 480,
- ov5640_setting_30fps_VGA_640_480,
- ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
- {OV5640_MODE_NTSC_720_480, SUBSAMPLING, 720, 480,
- ov5640_setting_30fps_NTSC_720_480,
- ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)},
- {OV5640_MODE_PAL_720_576, SUBSAMPLING, 720, 576,
- ov5640_setting_30fps_PAL_720_576,
- ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576)},
- {OV5640_MODE_XGA_1024_768, SUBSAMPLING, 1024, 768,
- ov5640_setting_30fps_XGA_1024_768,
- ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768)},
- {OV5640_MODE_720P_1280_720, SUBSAMPLING, 1280, 720,
- ov5640_setting_30fps_720P_1280_720,
- ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)},
- {OV5640_MODE_1080P_1920_1080, SCALING, 1920, 1080,
- ov5640_setting_30fps_1080P_1920_1080,
- ARRAY_SIZE(ov5640_setting_30fps_1080P_1920_1080)},
- {OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, NULL, 0},
+ .id = OV5640_MODE_VGA_640_480,
+ .dn_mode = SUBSAMPLING,
+ .width = 640,
+ .height = 480,
+ .reg_data = ov5640_setting_15fps_VGA_640_480,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)
},
+ {
+ .id = OV5640_MODE_NTSC_720_480,
+ .dn_mode = SUBSAMPLING,
+ .width = 720,
+ .height = 480,
+ .reg_data = ov5640_setting_15fps_NTSC_720_480,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480),
+ },
+ {
+ .id = OV5640_MODE_PAL_720_576,
+ .dn_mode = SUBSAMPLING,
+ .width = 720,
+ .height = 576,
+ .reg_data = ov5640_setting_15fps_PAL_720_576,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576),
+ },
+ {
+ .id = OV5640_MODE_XGA_1024_768,
+ .dn_mode = SUBSAMPLING,
+ .width = 1024,
+ .height = 768,
+ .reg_data = ov5640_setting_15fps_XGA_1024_768,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768),
+ },
+ {
+ .id = OV5640_MODE_720P_1280_720,
+ .dn_mode = SUBSAMPLING,
+ .width = 1280,
+ .height = 720,
+ .reg_data = ov5640_setting_15fps_720P_1280_720,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720),
+ },
+ {
+ .id = OV5640_MODE_1080P_1920_1080,
+ .dn_mode = SCALING,
+ .width = 1920,
+ .height = 1080,
+ .reg_data = ov5640_setting_15fps_1080P_1920_1080,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080),
+ },
+ {
+ .id = OV5640_MODE_QSXGA_2592_1944,
+ .dn_mode = SCALING,
+ .width = 2592,
+ .height = 1944,
+ .reg_data = ov5640_setting_15fps_QSXGA_2592_1944,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944),
+ },
+},
+{
+ {
+ .id = OV5640_MODE_QCIF_176_144,
+ .dn_mode = SUBSAMPLING,
+ .width = 176,
+ .height = 144,
+ .reg_data = ov5640_setting_30fps_QCIF_176_144,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144),
+ },
+ {
+ .id = OV5640_MODE_QVGA_320_240,
+ .dn_mode = SUBSAMPLING,
+ .width = 320,
+ .height = 240,
+ .reg_data = ov5640_setting_30fps_QVGA_320_240,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240),
+ },
+ {
+ .id = OV5640_MODE_VGA_640_480,
+ .dn_mode = SUBSAMPLING,
+ .width = 640,
+ .height = 480,
+ .reg_data = ov5640_setting_30fps_VGA_640_480,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480),
+ },
+ {
+ .id = OV5640_MODE_NTSC_720_480,
+ .dn_mode = SUBSAMPLING,
+ .width = 720,
+ .height = 480,
+ .reg_data = ov5640_setting_30fps_NTSC_720_480,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480),
+ },
+ {
+ .id = OV5640_MODE_PAL_720_576,
+ .dn_mode = SUBSAMPLING,
+ .width = 720,
+ .height = 576,
+ .reg_data = ov5640_setting_30fps_PAL_720_576,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576),
+ },
+ {
+ .id = OV5640_MODE_XGA_1024_768,
+ .dn_mode = SUBSAMPLING,
+ .width = 1024,
+ .height = 768,
+ .reg_data = ov5640_setting_30fps_XGA_1024_768,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768),
+ },
+ {
+ .id = OV5640_MODE_720P_1280_720,
+ .dn_mode = SUBSAMPLING,
+ .width = 1280,
+ .height = 720,
+ .reg_data = ov5640_setting_30fps_720P_1280_720,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720),
+ },
+ {
+ .id = OV5640_MODE_1080P_1920_1080,
+ .dn_mode = SCALING,
+ .width = 1920,
+ .height = 1080,
+ .reg_data = ov5640_setting_30fps_1080P_1920_1080,
+ .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_1080P_1920_1080),
+ },
+ {
+ .id = OV5640_MODE_QSXGA_2592_1944,
+ .dn_mode = -1,
+ .width = 0,
+ .height = 0,
+ .reg_data = NULL,
+ .reg_data_size = 0,
+ }
+}
};
static int ov5640_init_slave_id(struct ov5640_dev *sensor)
--
2.14.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls
2018-04-20 9:44 [PATCH 1/3] media: ov5640: initialize mode data structs by name Daniel Mack
@ 2018-04-20 9:44 ` Daniel Mack
2018-04-20 14:15 ` Maxime Ripard
2018-04-24 10:22 ` Sakari Ailus
2018-04-20 9:44 ` [PATCH 3/3] media: ov5640: add support for xclk frequency control Daniel Mack
1 sibling, 2 replies; 11+ messages in thread
From: Daniel Mack @ 2018-04-20 9:44 UTC (permalink / raw)
To: linux-media; +Cc: slongerbeam, mchehab, Daniel Mack
Add v4l2 controls to report the pixel and MIPI link rates of each mode.
The camss camera subsystem needs them to set up the correct hardware
clocks.
Tested on msm8016 based hardware.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
drivers/media/i2c/ov5640.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 96f1564abdf5..78669ed386cd 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -91,6 +91,20 @@
#define OV5640_REG_SDE_CTRL5 0x5585
#define OV5640_REG_AVG_READOUT 0x56a1
+#define OV5640_LINK_FREQ_111 0
+#define OV5640_LINK_FREQ_166 1
+#define OV5640_LINK_FREQ_222 2
+#define OV5640_LINK_FREQ_333 3
+#define OV5640_LINK_FREQ_666 4
+
+static const s64 link_freq_menu_items[] = {
+ 111066666,
+ 166600000,
+ 222133333,
+ 332200000,
+ 666400000,
+};
+
enum ov5640_mode_id {
OV5640_MODE_QCIF_176_144 = 0,
OV5640_MODE_QVGA_320_240,
@@ -167,12 +181,18 @@ struct ov5640_mode_info {
enum ov5640_downsize_mode dn_mode;
u32 width;
u32 height;
+ u32 pixel_rate;
+ u32 link_freq_idx;
const struct reg_value *reg_data;
u32 reg_data_size;
};
struct ov5640_ctrls {
struct v4l2_ctrl_handler handler;
+ struct {
+ struct v4l2_ctrl *link_freq;
+ struct v4l2_ctrl *pixel_rate;
+ };
struct {
struct v4l2_ctrl *auto_exp;
struct v4l2_ctrl *exposure;
@@ -732,6 +752,8 @@ static const struct ov5640_mode_info ov5640_mode_init_data = {
.dn_mode = SUBSAMPLING,
.width = 640,
.height = 480,
+ .pixel_rate = 27766666,
+ .link_freq_idx = OV5640_LINK_FREQ_111,
.reg_data = ov5640_init_setting_30fps_VGA,
.reg_data_size = ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
};
@@ -744,6 +766,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 176,
.height = 144,
+ .pixel_rate = 27766666,
+ .link_freq_idx = OV5640_LINK_FREQ_111,
.reg_data = ov5640_setting_15fps_QCIF_176_144,
.reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144),
},
@@ -752,6 +776,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 320,
.height = 240,
+ .pixel_rate = 27766666,
+ .link_freq_idx = OV5640_LINK_FREQ_111,
.reg_data = ov5640_setting_15fps_QVGA_320_240,
.reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240),
},
@@ -760,6 +786,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 640,
.height = 480,
+ .pixel_rate = 27766666,
+ .link_freq_idx = OV5640_LINK_FREQ_111,
.reg_data = ov5640_setting_15fps_VGA_640_480,
.reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)
},
@@ -768,6 +796,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 720,
.height = 480,
+ .pixel_rate = 27766666,
+ .link_freq_idx = OV5640_LINK_FREQ_111,
.reg_data = ov5640_setting_15fps_NTSC_720_480,
.reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480),
},
@@ -776,6 +806,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 720,
.height = 576,
+ .pixel_rate = 27766666,
+ .link_freq_idx = OV5640_LINK_FREQ_111,
.reg_data = ov5640_setting_15fps_PAL_720_576,
.reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576),
},
@@ -784,6 +816,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 1024,
.height = 768,
+ .pixel_rate = 27766666,
+ .link_freq_idx = OV5640_LINK_FREQ_111,
.reg_data = ov5640_setting_15fps_XGA_1024_768,
.reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768),
},
@@ -792,6 +826,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 1280,
.height = 720,
+ .pixel_rate = 41650000,
+ .link_freq_idx = OV5640_LINK_FREQ_166,
.reg_data = ov5640_setting_15fps_720P_1280_720,
.reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720),
},
@@ -800,6 +836,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SCALING,
.width = 1920,
.height = 1080,
+ .pixel_rate = 83300000,
+ .link_freq_idx = OV5640_LINK_FREQ_333,
.reg_data = ov5640_setting_15fps_1080P_1920_1080,
.reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080),
},
@@ -808,6 +846,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SCALING,
.width = 2592,
.height = 1944,
+ .pixel_rate = 83300000,
+ .link_freq_idx = OV5640_LINK_FREQ_333,
.reg_data = ov5640_setting_15fps_QSXGA_2592_1944,
.reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944),
},
@@ -818,6 +858,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 176,
.height = 144,
+ .pixel_rate = 27766666,
+ .link_freq_idx = OV5640_LINK_FREQ_111,
.reg_data = ov5640_setting_30fps_QCIF_176_144,
.reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144),
},
@@ -826,6 +868,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 320,
.height = 240,
+ .pixel_rate = 27766666,
+ .link_freq_idx = OV5640_LINK_FREQ_111,
.reg_data = ov5640_setting_30fps_QVGA_320_240,
.reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240),
},
@@ -834,6 +878,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 640,
.height = 480,
+ .pixel_rate = 27766666,
+ .link_freq_idx = OV5640_LINK_FREQ_111,
.reg_data = ov5640_setting_30fps_VGA_640_480,
.reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480),
},
@@ -842,6 +888,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 720,
.height = 480,
+ .pixel_rate = 55533333,
+ .link_freq_idx = OV5640_LINK_FREQ_222,
.reg_data = ov5640_setting_30fps_NTSC_720_480,
.reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480),
},
@@ -850,6 +898,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 720,
.height = 576,
+ .pixel_rate = 55533333,
+ .link_freq_idx = OV5640_LINK_FREQ_222,
.reg_data = ov5640_setting_30fps_PAL_720_576,
.reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576),
},
@@ -858,6 +908,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 1024,
.height = 768,
+ .pixel_rate = 55533333,
+ .link_freq_idx = OV5640_LINK_FREQ_222,
.reg_data = ov5640_setting_30fps_XGA_1024_768,
.reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768),
},
@@ -866,6 +918,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SUBSAMPLING,
.width = 1280,
.height = 720,
+ .pixel_rate = 83300000,
+ .link_freq_idx = OV5640_LINK_FREQ_333,
.reg_data = ov5640_setting_30fps_720P_1280_720,
.reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720),
},
@@ -874,6 +928,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = SCALING,
.width = 1920,
.height = 1080,
+ .pixel_rate = 166600000,
+ .link_freq_idx = OV5640_LINK_FREQ_666,
.reg_data = ov5640_setting_30fps_1080P_1920_1080,
.reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_1080P_1920_1080),
},
@@ -882,6 +938,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
.dn_mode = -1,
.width = 0,
.height = 0,
+ .pixel_rate = 0,
+ .link_freq_idx = 0,
.reg_data = NULL,
.reg_data_size = 0,
}
@@ -2024,6 +2082,11 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
sensor->current_mode = new_mode;
sensor->fmt = *mbus_fmt;
sensor->pending_mode_change = true;
+
+ __v4l2_ctrl_s_ctrl(sensor->ctrls.link_freq, new_mode->link_freq_idx);
+ __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
+ new_mode->pixel_rate);
+
out:
mutex_unlock(&sensor->lock);
return ret;
@@ -2350,6 +2413,20 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
/* we can use our own mutex for the ctrl lock */
hdl->lock = &sensor->lock;
+ /* Clock related controls */
+ ctrls->link_freq =
+ v4l2_ctrl_new_int_menu(hdl, ops,
+ V4L2_CID_LINK_FREQ,
+ ARRAY_SIZE(link_freq_menu_items) - 1,
+ 0, link_freq_menu_items);
+ ctrls->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+ ctrls->pixel_rate =
+ v4l2_ctrl_new_std(hdl, ops,
+ V4L2_CID_PIXEL_RATE, 0, INT_MAX, 1,
+ ov5640_mode_init_data.pixel_rate);
+ ctrls->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
/* Auto/manual white balance */
ctrls->auto_wb = v4l2_ctrl_new_std(hdl, ops,
V4L2_CID_AUTO_WHITE_BALANCE,
--
2.14.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls
2018-04-20 9:44 ` [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls Daniel Mack
@ 2018-04-20 14:15 ` Maxime Ripard
2018-04-20 14:29 ` Daniel Mack
2018-04-24 10:22 ` Sakari Ailus
1 sibling, 1 reply; 11+ messages in thread
From: Maxime Ripard @ 2018-04-20 14:15 UTC (permalink / raw)
To: Daniel Mack; +Cc: linux-media, slongerbeam, mchehab
[-- Attachment #1: Type: text/plain, Size: 2495 bytes --]
Hi,
On Fri, Apr 20, 2018 at 11:44:18AM +0200, Daniel Mack wrote:
> Add v4l2 controls to report the pixel and MIPI link rates of each mode.
> The camss camera subsystem needs them to set up the correct hardware
> clocks.
>
> Tested on msm8016 based hardware.
>
> Signed-off-by: Daniel Mack <daniel@zonque.org>
> ---
> drivers/media/i2c/ov5640.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 77 insertions(+)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 96f1564abdf5..78669ed386cd 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -91,6 +91,20 @@
> #define OV5640_REG_SDE_CTRL5 0x5585
> #define OV5640_REG_AVG_READOUT 0x56a1
>
> +#define OV5640_LINK_FREQ_111 0
> +#define OV5640_LINK_FREQ_166 1
> +#define OV5640_LINK_FREQ_222 2
> +#define OV5640_LINK_FREQ_333 3
> +#define OV5640_LINK_FREQ_666 4
> +
> +static const s64 link_freq_menu_items[] = {
> + 111066666,
> + 166600000,
> + 222133333,
> + 332200000,
> + 666400000,
> +};
> +
> enum ov5640_mode_id {
> OV5640_MODE_QCIF_176_144 = 0,
> OV5640_MODE_QVGA_320_240,
> @@ -167,12 +181,18 @@ struct ov5640_mode_info {
> enum ov5640_downsize_mode dn_mode;
> u32 width;
> u32 height;
> + u32 pixel_rate;
> + u32 link_freq_idx;
> const struct reg_value *reg_data;
> u32 reg_data_size;
> };
>
> struct ov5640_ctrls {
> struct v4l2_ctrl_handler handler;
> + struct {
> + struct v4l2_ctrl *link_freq;
> + struct v4l2_ctrl *pixel_rate;
> + };
> struct {
> struct v4l2_ctrl *auto_exp;
> struct v4l2_ctrl *exposure;
> @@ -732,6 +752,8 @@ static const struct ov5640_mode_info ov5640_mode_init_data = {
> .dn_mode = SUBSAMPLING,
> .width = 640,
> .height = 480,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
I'm not sure where this is coming from, but on a parallel sensor I
have a quite different pixel rate.
I have a serie ongoing that tries to deal with this, hopefully in
order to get rid of all the clock setup done in the initialiasation
array.
See https://patchwork.linuxtv.org/patch/48710/ for the patch and
https://www.spinics.net/lists/linux-media/msg132201.html for a
discussion on what the clock tree might look like on a MIPI-CSI bus.
Feel free to step in the discussion.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls
2018-04-20 14:15 ` Maxime Ripard
@ 2018-04-20 14:29 ` Daniel Mack
2018-04-20 19:00 ` Maxime Ripard
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Mack @ 2018-04-20 14:29 UTC (permalink / raw)
To: Maxime Ripard; +Cc: linux-media, slongerbeam, mchehab
Hi,
On Friday, April 20, 2018 04:15 PM, Maxime Ripard wrote:
> On Fri, Apr 20, 2018 at 11:44:18AM +0200, Daniel Mack wrote:
>> struct ov5640_ctrls {
>> struct v4l2_ctrl_handler handler;
>> + struct {
>> + struct v4l2_ctrl *link_freq;
>> + struct v4l2_ctrl *pixel_rate;
>> + };
>> struct {
>> struct v4l2_ctrl *auto_exp;
>> struct v4l2_ctrl *exposure;
>> @@ -732,6 +752,8 @@ static const struct ov5640_mode_info ov5640_mode_init_data = {
>> .dn_mode = SUBSAMPLING,
>> .width = 640,
>> .height = 480,
>> + .pixel_rate = 27766666,
>> + .link_freq_idx = OV5640_LINK_FREQ_111,
>
> I'm not sure where this is coming from, but on a parallel sensor I
> have a quite different pixel rate.
Ah, interesting. What exactly do you mean by 'parallel'? What kind of
module is that, and what are your pixel rates?
> I have a serie ongoing that tries to deal with this, hopefully in
> order to get rid of all the clock setup done in the initialiasation
> array.
>
> See https://patchwork.linuxtv.org/patch/48710/ for the patch and
> https://www.spinics.net/lists/linux-media/msg132201.html for a
> discussion on what the clock tree might look like on a MIPI-CSI bus.
Okay, nice. Even better if this patch isn't needed in the end.
Thanks!
Daniel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls
2018-04-20 14:29 ` Daniel Mack
@ 2018-04-20 19:00 ` Maxime Ripard
0 siblings, 0 replies; 11+ messages in thread
From: Maxime Ripard @ 2018-04-20 19:00 UTC (permalink / raw)
To: Daniel Mack; +Cc: linux-media, slongerbeam, mchehab
On Fri, Apr 20, 2018 at 04:29:42PM +0200, Daniel Mack wrote:
> Hi,
>
> On Friday, April 20, 2018 04:15 PM, Maxime Ripard wrote:
> > On Fri, Apr 20, 2018 at 11:44:18AM +0200, Daniel Mack wrote:
>
> >> struct ov5640_ctrls {
> >> struct v4l2_ctrl_handler handler;
> >> + struct {
> >> + struct v4l2_ctrl *link_freq;
> >> + struct v4l2_ctrl *pixel_rate;
> >> + };
> >> struct {
> >> struct v4l2_ctrl *auto_exp;
> >> struct v4l2_ctrl *exposure;
> >> @@ -732,6 +752,8 @@ static const struct ov5640_mode_info ov5640_mode_init_data = {
> >> .dn_mode = SUBSAMPLING,
> >> .width = 640,
> >> .height = 480,
> >> + .pixel_rate = 27766666,
> >> + .link_freq_idx = OV5640_LINK_FREQ_111,
> >
> > I'm not sure where this is coming from, but on a parallel sensor I
> > have a quite different pixel rate.
>
> Ah, interesting. What exactly do you mean by 'parallel'? What kind of
> module is that, and what are your pixel rates?
An RGB bus, or MIPI-DPI, or basically a pixel clock + 1 line for each
bit. The sensor can operate using both that bus and a MIPI-CSI2 one.
You have the list of pixel rates in the patch I've linked before:
https://patchwork.linuxtv.org/patch/48710/
There's one pixel sent per clock cycle, so the pixel rate is the same
than the clock rate.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls
2018-04-20 9:44 ` [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls Daniel Mack
2018-04-20 14:15 ` Maxime Ripard
@ 2018-04-24 10:22 ` Sakari Ailus
2018-04-24 12:03 ` Daniel Mack
1 sibling, 1 reply; 11+ messages in thread
From: Sakari Ailus @ 2018-04-24 10:22 UTC (permalink / raw)
To: Daniel Mack; +Cc: linux-media, slongerbeam, mchehab
Hi Daniel,
On Fri, Apr 20, 2018 at 11:44:18AM +0200, Daniel Mack wrote:
> Add v4l2 controls to report the pixel and MIPI link rates of each mode.
> The camss camera subsystem needs them to set up the correct hardware
> clocks.
>
> Tested on msm8016 based hardware.
>
> Signed-off-by: Daniel Mack <daniel@zonque.org>
Maxime has written a number of patches against the driver that seem very
much related; could you rebase these on his set (v2)?
<URL:https://patchwork.linuxtv.org/project/linux-media/list/?submitter=Maxime+Ripard&state=*&q=ov5640>
> ---
> drivers/media/i2c/ov5640.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 77 insertions(+)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 96f1564abdf5..78669ed386cd 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -91,6 +91,20 @@
> #define OV5640_REG_SDE_CTRL5 0x5585
> #define OV5640_REG_AVG_READOUT 0x56a1
>
> +#define OV5640_LINK_FREQ_111 0
> +#define OV5640_LINK_FREQ_166 1
> +#define OV5640_LINK_FREQ_222 2
> +#define OV5640_LINK_FREQ_333 3
> +#define OV5640_LINK_FREQ_666 4
> +
> +static const s64 link_freq_menu_items[] = {
> + 111066666,
> + 166600000,
> + 222133333,
> + 332200000,
> + 666400000,
> +};
> +
> enum ov5640_mode_id {
> OV5640_MODE_QCIF_176_144 = 0,
> OV5640_MODE_QVGA_320_240,
> @@ -167,12 +181,18 @@ struct ov5640_mode_info {
> enum ov5640_downsize_mode dn_mode;
> u32 width;
> u32 height;
> + u32 pixel_rate;
> + u32 link_freq_idx;
> const struct reg_value *reg_data;
> u32 reg_data_size;
> };
>
> struct ov5640_ctrls {
> struct v4l2_ctrl_handler handler;
> + struct {
> + struct v4l2_ctrl *link_freq;
> + struct v4l2_ctrl *pixel_rate;
> + };
> struct {
> struct v4l2_ctrl *auto_exp;
> struct v4l2_ctrl *exposure;
> @@ -732,6 +752,8 @@ static const struct ov5640_mode_info ov5640_mode_init_data = {
> .dn_mode = SUBSAMPLING,
> .width = 640,
> .height = 480,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
> .reg_data = ov5640_init_setting_30fps_VGA,
> .reg_data_size = ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
> };
> @@ -744,6 +766,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 176,
> .height = 144,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
> .reg_data = ov5640_setting_15fps_QCIF_176_144,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144),
> },
> @@ -752,6 +776,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 320,
> .height = 240,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
> .reg_data = ov5640_setting_15fps_QVGA_320_240,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240),
> },
> @@ -760,6 +786,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 640,
> .height = 480,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
> .reg_data = ov5640_setting_15fps_VGA_640_480,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)
> },
> @@ -768,6 +796,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 720,
> .height = 480,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
> .reg_data = ov5640_setting_15fps_NTSC_720_480,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480),
> },
> @@ -776,6 +806,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 720,
> .height = 576,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
> .reg_data = ov5640_setting_15fps_PAL_720_576,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576),
> },
> @@ -784,6 +816,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 1024,
> .height = 768,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
> .reg_data = ov5640_setting_15fps_XGA_1024_768,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768),
> },
> @@ -792,6 +826,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 1280,
> .height = 720,
> + .pixel_rate = 41650000,
> + .link_freq_idx = OV5640_LINK_FREQ_166,
> .reg_data = ov5640_setting_15fps_720P_1280_720,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720),
> },
> @@ -800,6 +836,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SCALING,
> .width = 1920,
> .height = 1080,
> + .pixel_rate = 83300000,
> + .link_freq_idx = OV5640_LINK_FREQ_333,
> .reg_data = ov5640_setting_15fps_1080P_1920_1080,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080),
> },
> @@ -808,6 +846,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SCALING,
> .width = 2592,
> .height = 1944,
> + .pixel_rate = 83300000,
> + .link_freq_idx = OV5640_LINK_FREQ_333,
> .reg_data = ov5640_setting_15fps_QSXGA_2592_1944,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944),
> },
> @@ -818,6 +858,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 176,
> .height = 144,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
> .reg_data = ov5640_setting_30fps_QCIF_176_144,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144),
> },
> @@ -826,6 +868,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 320,
> .height = 240,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
> .reg_data = ov5640_setting_30fps_QVGA_320_240,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240),
> },
> @@ -834,6 +878,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 640,
> .height = 480,
> + .pixel_rate = 27766666,
> + .link_freq_idx = OV5640_LINK_FREQ_111,
> .reg_data = ov5640_setting_30fps_VGA_640_480,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480),
> },
> @@ -842,6 +888,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 720,
> .height = 480,
> + .pixel_rate = 55533333,
> + .link_freq_idx = OV5640_LINK_FREQ_222,
> .reg_data = ov5640_setting_30fps_NTSC_720_480,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480),
> },
> @@ -850,6 +898,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 720,
> .height = 576,
> + .pixel_rate = 55533333,
> + .link_freq_idx = OV5640_LINK_FREQ_222,
> .reg_data = ov5640_setting_30fps_PAL_720_576,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576),
> },
> @@ -858,6 +908,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 1024,
> .height = 768,
> + .pixel_rate = 55533333,
> + .link_freq_idx = OV5640_LINK_FREQ_222,
> .reg_data = ov5640_setting_30fps_XGA_1024_768,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768),
> },
> @@ -866,6 +918,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SUBSAMPLING,
> .width = 1280,
> .height = 720,
> + .pixel_rate = 83300000,
> + .link_freq_idx = OV5640_LINK_FREQ_333,
> .reg_data = ov5640_setting_30fps_720P_1280_720,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720),
> },
> @@ -874,6 +928,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = SCALING,
> .width = 1920,
> .height = 1080,
> + .pixel_rate = 166600000,
> + .link_freq_idx = OV5640_LINK_FREQ_666,
> .reg_data = ov5640_setting_30fps_1080P_1920_1080,
> .reg_data_size = ARRAY_SIZE(ov5640_setting_30fps_1080P_1920_1080),
> },
> @@ -882,6 +938,8 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> .dn_mode = -1,
> .width = 0,
> .height = 0,
> + .pixel_rate = 0,
> + .link_freq_idx = 0,
> .reg_data = NULL,
> .reg_data_size = 0,
> }
> @@ -2024,6 +2082,11 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
> sensor->current_mode = new_mode;
> sensor->fmt = *mbus_fmt;
> sensor->pending_mode_change = true;
> +
> + __v4l2_ctrl_s_ctrl(sensor->ctrls.link_freq, new_mode->link_freq_idx);
> + __v4l2_ctrl_s_ctrl_int64(sensor->ctrls.pixel_rate,
> + new_mode->pixel_rate);
> +
> out:
> mutex_unlock(&sensor->lock);
> return ret;
> @@ -2350,6 +2413,20 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
> /* we can use our own mutex for the ctrl lock */
> hdl->lock = &sensor->lock;
>
> + /* Clock related controls */
> + ctrls->link_freq =
> + v4l2_ctrl_new_int_menu(hdl, ops,
> + V4L2_CID_LINK_FREQ,
> + ARRAY_SIZE(link_freq_menu_items) - 1,
> + 0, link_freq_menu_items);
> + ctrls->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
Do make sure creating the control succeeds, i.e. check that
ctrls->link_freq is non-NULL before referencing it. Same below.
> +
> + ctrls->pixel_rate =
> + v4l2_ctrl_new_std(hdl, ops,
> + V4L2_CID_PIXEL_RATE, 0, INT_MAX, 1,
> + ov5640_mode_init_data.pixel_rate);
> + ctrls->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> +
> /* Auto/manual white balance */
> ctrls->auto_wb = v4l2_ctrl_new_std(hdl, ops,
> V4L2_CID_AUTO_WHITE_BALANCE,
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls
2018-04-24 10:22 ` Sakari Ailus
@ 2018-04-24 12:03 ` Daniel Mack
2018-04-26 8:31 ` Sakari Ailus
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Mack @ 2018-04-24 12:03 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, slongerbeam, mchehab
Hi,
On Tuesday, April 24, 2018 12:22 PM, Sakari Ailus wrote:
> On Fri, Apr 20, 2018 at 11:44:18AM +0200, Daniel Mack wrote:
>> Add v4l2 controls to report the pixel and MIPI link rates of each mode.
>> The camss camera subsystem needs them to set up the correct hardware
>> clocks.
>>
>> Tested on msm8016 based hardware.
>>
>> Signed-off-by: Daniel Mack <daniel@zonque.org>
>
> Maxime has written a number of patches against the driver that seem very
> much related; could you rebase these on his set (v2)?
>
> <URL:https://patchwork.linuxtv.org/project/linux-media/list/?submitter=Maxime+Ripard&state=*&q=ov5640>
I didn't know about the ongoing work in this area, so I think both this
and 3/3 are not needed. If you want, you can still pick the 1st patch in
this series, but that's just a cosmetic cleanup.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls
2018-04-24 12:03 ` Daniel Mack
@ 2018-04-26 8:31 ` Sakari Ailus
0 siblings, 0 replies; 11+ messages in thread
From: Sakari Ailus @ 2018-04-26 8:31 UTC (permalink / raw)
To: Daniel Mack; +Cc: linux-media, slongerbeam, mchehab
On Tue, Apr 24, 2018 at 02:03:22PM +0200, Daniel Mack wrote:
> Hi,
>
> On Tuesday, April 24, 2018 12:22 PM, Sakari Ailus wrote:
> > On Fri, Apr 20, 2018 at 11:44:18AM +0200, Daniel Mack wrote:
> >> Add v4l2 controls to report the pixel and MIPI link rates of each mode.
> >> The camss camera subsystem needs them to set up the correct hardware
> >> clocks.
> >>
> >> Tested on msm8016 based hardware.
> >>
> >> Signed-off-by: Daniel Mack <daniel@zonque.org>
> >
> > Maxime has written a number of patches against the driver that seem very
> > much related; could you rebase these on his set (v2)?
> >
> > <URL:https://patchwork.linuxtv.org/project/linux-media/list/?submitter=Maxime+Ripard&state=*&q=ov5640>
>
> I didn't know about the ongoing work in this area, so I think both this
> and 3/3 are not needed. If you want, you can still pick the 1st patch in
> this series, but that's just a cosmetic cleanup.
That patch, too, would effectively need a rebase.
I'd also suggest adding a macro that constructs the entries in the array
--- much of what changes from entry to entry are fps, width, height and
whether subsampling or scaling has been used. That information would likely
fit nicely on a single line.
The resolution names are also redundant as the size is explicitly part of
the mode list variable names.
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] media: ov5640: add support for xclk frequency control
2018-04-20 9:44 [PATCH 1/3] media: ov5640: initialize mode data structs by name Daniel Mack
2018-04-20 9:44 ` [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls Daniel Mack
@ 2018-04-20 9:44 ` Daniel Mack
2018-04-20 14:07 ` Maxime Ripard
2018-04-24 10:24 ` Sakari Ailus
1 sibling, 2 replies; 11+ messages in thread
From: Daniel Mack @ 2018-04-20 9:44 UTC (permalink / raw)
To: linux-media; +Cc: slongerbeam, mchehab, Daniel Mack
Allow setting the xclk rate via an optional 'clock-frequency' property in
the device tree node.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
Documentation/devicetree/bindings/media/i2c/ov5640.txt | 2 ++
drivers/media/i2c/ov5640.c | 10 ++++++++++
2 files changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
index 8e36da0d8406..584bbc944978 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
@@ -13,6 +13,8 @@ Optional Properties:
This is an active low signal to the OV5640.
- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
if any. This is an active high signal to the OV5640.
+- clock-frequency: frequency to set on the xclk input clock. The clock
+ is left untouched if this property is missing.
The device node must contain one 'port' child node for its digital output
video port, in accordance with the video interface bindings defined in
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 78669ed386cd..2d94d6dbda5d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -2685,6 +2685,7 @@ static int ov5640_probe(struct i2c_client *client,
struct fwnode_handle *endpoint;
struct ov5640_dev *sensor;
struct v4l2_mbus_framefmt *fmt;
+ u32 freq;
int ret;
sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
@@ -2731,6 +2732,15 @@ static int ov5640_probe(struct i2c_client *client,
return PTR_ERR(sensor->xclk);
}
+ ret = of_property_read_u32(dev->of_node, "clock-frequency", &freq);
+ if (ret == 0) {
+ ret = clk_set_rate(sensor->xclk, freq);
+ if (ret) {
+ dev_err(dev, "could not set xclk frequency\n");
+ return ret;
+ }
+ }
+
sensor->xclk_freq = clk_get_rate(sensor->xclk);
if (sensor->xclk_freq < OV5640_XCLK_MIN ||
sensor->xclk_freq > OV5640_XCLK_MAX) {
--
2.14.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/3] media: ov5640: add support for xclk frequency control
2018-04-20 9:44 ` [PATCH 3/3] media: ov5640: add support for xclk frequency control Daniel Mack
@ 2018-04-20 14:07 ` Maxime Ripard
2018-04-24 10:24 ` Sakari Ailus
1 sibling, 0 replies; 11+ messages in thread
From: Maxime Ripard @ 2018-04-20 14:07 UTC (permalink / raw)
To: Daniel Mack; +Cc: linux-media, slongerbeam, mchehab
[-- Attachment #1: Type: text/plain, Size: 2540 bytes --]
Hi,
On Fri, Apr 20, 2018 at 11:44:19AM +0200, Daniel Mack wrote:
> Allow setting the xclk rate via an optional 'clock-frequency' property in
> the device tree node.
>
> Signed-off-by: Daniel Mack <daniel@zonque.org>
> ---
> Documentation/devicetree/bindings/media/i2c/ov5640.txt | 2 ++
> drivers/media/i2c/ov5640.c | 10 ++++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> index 8e36da0d8406..584bbc944978 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> @@ -13,6 +13,8 @@ Optional Properties:
> This is an active low signal to the OV5640.
> - powerdown-gpios: reference to the GPIO connected to the powerdown pin,
> if any. This is an active high signal to the OV5640.
> +- clock-frequency: frequency to set on the xclk input clock. The clock
> + is left untouched if this property is missing.
This can be done through assigned-clocks, right?
> The device node must contain one 'port' child node for its digital output
> video port, in accordance with the video interface bindings defined in
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 78669ed386cd..2d94d6dbda5d 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2685,6 +2685,7 @@ static int ov5640_probe(struct i2c_client *client,
> struct fwnode_handle *endpoint;
> struct ov5640_dev *sensor;
> struct v4l2_mbus_framefmt *fmt;
> + u32 freq;
> int ret;
>
> sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
> @@ -2731,6 +2732,15 @@ static int ov5640_probe(struct i2c_client *client,
> return PTR_ERR(sensor->xclk);
> }
>
> + ret = of_property_read_u32(dev->of_node, "clock-frequency", &freq);
> + if (ret == 0) {
> + ret = clk_set_rate(sensor->xclk, freq);
> + if (ret) {
> + dev_err(dev, "could not set xclk frequency\n");
> + return ret;
> + }
> + }
> +
I'm wondering what the use case for that would be. The clock rate is
subject to various changes depending on the resolution and framerate
used, so that's very likely to change. Wouldn't we be better off to
simply try to change the rate at runtime, depending on those factors?
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/3] media: ov5640: add support for xclk frequency control
2018-04-20 9:44 ` [PATCH 3/3] media: ov5640: add support for xclk frequency control Daniel Mack
2018-04-20 14:07 ` Maxime Ripard
@ 2018-04-24 10:24 ` Sakari Ailus
1 sibling, 0 replies; 11+ messages in thread
From: Sakari Ailus @ 2018-04-24 10:24 UTC (permalink / raw)
To: Daniel Mack; +Cc: linux-media, slongerbeam, mchehab
On Fri, Apr 20, 2018 at 11:44:19AM +0200, Daniel Mack wrote:
> Allow setting the xclk rate via an optional 'clock-frequency' property in
> the device tree node.
>
> Signed-off-by: Daniel Mack <daniel@zonque.org>
> ---
> Documentation/devicetree/bindings/media/i2c/ov5640.txt | 2 ++
> drivers/media/i2c/ov5640.c | 10 ++++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> index 8e36da0d8406..584bbc944978 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> @@ -13,6 +13,8 @@ Optional Properties:
> This is an active low signal to the OV5640.
> - powerdown-gpios: reference to the GPIO connected to the powerdown pin,
> if any. This is an active high signal to the OV5640.
> +- clock-frequency: frequency to set on the xclk input clock. The clock
> + is left untouched if this property is missing.
>
> The device node must contain one 'port' child node for its digital output
> video port, in accordance with the video interface bindings defined in
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 78669ed386cd..2d94d6dbda5d 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2685,6 +2685,7 @@ static int ov5640_probe(struct i2c_client *client,
> struct fwnode_handle *endpoint;
> struct ov5640_dev *sensor;
> struct v4l2_mbus_framefmt *fmt;
> + u32 freq;
> int ret;
>
> sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
> @@ -2731,6 +2732,15 @@ static int ov5640_probe(struct i2c_client *client,
> return PTR_ERR(sensor->xclk);
> }
>
> + ret = of_property_read_u32(dev->of_node, "clock-frequency", &freq);
Please use
ret = fwnode_property_read_u32(dev_fwnode(dev), ...);
Thanks.
> + if (ret == 0) {
> + ret = clk_set_rate(sensor->xclk, freq);
> + if (ret) {
> + dev_err(dev, "could not set xclk frequency\n");
> + return ret;
> + }
> + }
> +
> sensor->xclk_freq = clk_get_rate(sensor->xclk);
> if (sensor->xclk_freq < OV5640_XCLK_MIN ||
> sensor->xclk_freq > OV5640_XCLK_MAX) {
> --
> 2.14.3
>
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-04-26 8:31 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-20 9:44 [PATCH 1/3] media: ov5640: initialize mode data structs by name Daniel Mack
2018-04-20 9:44 ` [PATCH 2/3] media: ov5640: add PIXEL_RATE and LINK_FREQ controls Daniel Mack
2018-04-20 14:15 ` Maxime Ripard
2018-04-20 14:29 ` Daniel Mack
2018-04-20 19:00 ` Maxime Ripard
2018-04-24 10:22 ` Sakari Ailus
2018-04-24 12:03 ` Daniel Mack
2018-04-26 8:31 ` Sakari Ailus
2018-04-20 9:44 ` [PATCH 3/3] media: ov5640: add support for xclk frequency control Daniel Mack
2018-04-20 14:07 ` Maxime Ripard
2018-04-24 10:24 ` Sakari Ailus
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.