* [PATCH 0/2] OV5640 hflip, vflip and module orientation support
@ 2018-06-11 9:29 ` Hugues Fruchet
0 siblings, 0 replies; 17+ messages in thread
From: Hugues Fruchet @ 2018-06-11 9:29 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Hans Verkuil,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Rob Herring, Mark Rutland
Cc: devicetree, linux-media,
Hugues Fruchet <hugues.fruchet@st.com>,
Benjamin Gaignard, Maxime Ripard
This patch serie relates to flip and mirror at sensor level through
registers "TIMING TC REG20/REG21".
The first commit implements HFLIP and VFLIP V4L2 controls
allowing V4L2 client to change horizontal and vertical flip
before or during streaming.
The second commit allows to inform driver of the physical
orientation of the sensor module through devicetree "rotation"
optional properties as defined by Sakari in media/video-interfaces.txt:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg132345.html
Hugues Fruchet (2):
media: ov5640: add HFLIP/VFLIP controls support
media: ov5640: add support of module orientation
.../devicetree/bindings/media/i2c/ov5640.txt | 3 +
drivers/media/i2c/ov5640.c | 127 ++++++++++++++++++---
2 files changed, 112 insertions(+), 18 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 0/2] OV5640 hflip, vflip and module orientation support
@ 2018-06-11 9:29 ` Hugues Fruchet
0 siblings, 0 replies; 17+ messages in thread
From: Hugues Fruchet @ 2018-06-11 9:29 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Hans Verkuil,
Mauro Carvalho Chehab, Rob Herring, Mark Rutland
Cc: devicetree, linux-media, Hugues Fruchet, Benjamin Gaignard,
Maxime Ripard
This patch serie relates to flip and mirror at sensor level through
registers "TIMING TC REG20/REG21".
The first commit implements HFLIP and VFLIP V4L2 controls
allowing V4L2 client to change horizontal and vertical flip
before or during streaming.
The second commit allows to inform driver of the physical
orientation of the sensor module through devicetree "rotation"
optional properties as defined by Sakari in media/video-interfaces.txt:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg132345.html
Hugues Fruchet (2):
media: ov5640: add HFLIP/VFLIP controls support
media: ov5640: add support of module orientation
.../devicetree/bindings/media/i2c/ov5640.txt | 3 +
drivers/media/i2c/ov5640.c | 127 ++++++++++++++++++---
2 files changed, 112 insertions(+), 18 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] media: ov5640: add HFLIP/VFLIP controls support
2018-06-11 9:29 ` Hugues Fruchet
@ 2018-06-11 9:29 ` Hugues Fruchet
-1 siblings, 0 replies; 17+ messages in thread
From: Hugues Fruchet @ 2018-06-11 9:29 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Hans Verkuil,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Rob Herring, Mark Rutland
Cc: devicetree, linux-media,
Hugues Fruchet <hugues.fruchet@st.com>,
Benjamin Gaignard, Maxime Ripard
Add HFLIP/VFLIP controls support by setting registers REG21/REG20.
Useless values in hardcoded mode sequences are removed and
remaining binning values are now set after mode sequence being set.
Note that due to BSI (Back Side Illuminated) technology, image capture
is physically mirrored, mirror logic is so inversed in REG21 register
to cancel this effect.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
drivers/media/i2c/ov5640.c | 103 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 85 insertions(+), 18 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index f6e40cc..41039e5 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -64,6 +64,7 @@
#define OV5640_REG_TIMING_DVPVO 0x380a
#define OV5640_REG_TIMING_HTS 0x380c
#define OV5640_REG_TIMING_VTS 0x380e
+#define OV5640_REG_TIMING_TC_REG20 0x3820
#define OV5640_REG_TIMING_TC_REG21 0x3821
#define OV5640_REG_AEC_CTRL00 0x3a00
#define OV5640_REG_AEC_B50_STEP 0x3a08
@@ -199,6 +200,8 @@ struct ov5640_ctrls {
struct v4l2_ctrl *contrast;
struct v4l2_ctrl *hue;
struct v4l2_ctrl *test_pattern;
+ struct v4l2_ctrl *hflip;
+ struct v4l2_ctrl *vflip;
};
struct ov5640_dev {
@@ -341,7 +344,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -360,7 +363,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -379,7 +382,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -399,7 +402,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -418,7 +421,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -437,7 +440,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -456,7 +459,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -475,7 +478,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -494,7 +497,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_NTSC_720_480[] = {
{0x3035, 0x12, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -513,7 +516,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_NTSC_720_480[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -532,7 +535,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_PAL_720_576[] = {
{0x3035, 0x12, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -551,7 +554,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_PAL_720_576[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -571,7 +574,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
{0x3008, 0x42, 0, 0},
{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x07, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
@@ -591,7 +594,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_720P_1280_720[] = {
{0x3035, 0x41, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x07, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
@@ -611,7 +614,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
{0x3008, 0x42, 0, 0},
{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
+ {0x3814, 0x11, 0, 0},
{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
@@ -644,7 +647,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
{0x3008, 0x42, 0, 0},
{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
+ {0x3814, 0x11, 0, 0},
{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
@@ -673,10 +676,9 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
};
static const struct reg_value ov5640_setting_15fps_QSXGA_2592_1944[] = {
- {0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0},
{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
+ {0x3814, 0x11, 0, 0},
{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
@@ -1340,6 +1342,27 @@ static int ov5640_binning_on(struct ov5640_dev *sensor)
return temp ? 1 : 0;
}
+static int ov5640_set_binning(struct ov5640_dev *sensor, bool enable)
+{
+ int ret;
+
+ /*
+ * TIMING TC REG21:
+ * - [0]: Horizontal binning enable
+ */
+ ret = ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
+ BIT(0), enable ? BIT(0) : 0);
+ if (ret)
+ return ret;
+ /*
+ * TIMING TC REG20:
+ * - [0]: Undocumented, but hardcoded init sequences
+ * are always setting REG21/REG20 bit 0 to same value...
+ */
+ return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
+ BIT(0), enable ? BIT(0) : 0);
+}
+
static int ov5640_set_virtual_channel(struct ov5640_dev *sensor)
{
struct i2c_client *client = sensor->i2c_client;
@@ -1640,6 +1663,9 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
if (ret < 0)
return ret;
+ ret = ov5640_set_binning(sensor, dn_mode != SCALING);
+ if (ret < 0)
+ return ret;
ret = ov5640_set_ae_target(sensor, sensor->ae_target);
if (ret < 0)
return ret;
@@ -2193,6 +2219,37 @@ static int ov5640_set_ctrl_light_freq(struct ov5640_dev *sensor, int value)
BIT(2) : 0);
}
+static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
+{
+ /*
+ * Sensor is a BSI (Back Side Illuminated) one,
+ * so image captured is physically mirrored.
+ * This is why mirror logic is inversed in
+ * order to cancel this mirror effect.
+ */
+
+ /*
+ * TIMING TC REG21:
+ * - [2]: ISP mirror
+ * - [1]: Sensor mirror
+ */
+ return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
+ BIT(2) | BIT(1),
+ (!value) ? (BIT(2) | BIT(1)) : 0);
+}
+
+static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
+{
+ /*
+ * TIMING TC REG20:
+ * - [2]: ISP vflip
+ * - [1]: Sensor vflip
+ */
+ return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
+ BIT(2) | BIT(1),
+ value ? (BIT(2) | BIT(1)) : 0);
+}
+
static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{
struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
@@ -2264,6 +2321,12 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_POWER_LINE_FREQUENCY:
ret = ov5640_set_ctrl_light_freq(sensor, ctrl->val);
break;
+ case V4L2_CID_HFLIP:
+ ret = ov5640_set_ctrl_hflip(sensor, ctrl->val);
+ break;
+ case V4L2_CID_VFLIP:
+ ret = ov5640_set_ctrl_vflip(sensor, ctrl->val);
+ break;
default:
ret = -EINVAL;
break;
@@ -2325,6 +2388,10 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_TEST_PATTERN,
ARRAY_SIZE(test_pattern_menu) - 1,
0, 0, test_pattern_menu);
+ ctrls->hflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP,
+ 0, 1, 1, 0);
+ ctrls->vflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP,
+ 0, 1, 1, 0);
ctrls->light_freq =
v4l2_ctrl_new_std_menu(hdl, ops,
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 1/2] media: ov5640: add HFLIP/VFLIP controls support
@ 2018-06-11 9:29 ` Hugues Fruchet
0 siblings, 0 replies; 17+ messages in thread
From: Hugues Fruchet @ 2018-06-11 9:29 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Hans Verkuil,
Mauro Carvalho Chehab, Rob Herring, Mark Rutland
Cc: devicetree, linux-media, Hugues Fruchet, Benjamin Gaignard,
Maxime Ripard
Add HFLIP/VFLIP controls support by setting registers REG21/REG20.
Useless values in hardcoded mode sequences are removed and
remaining binning values are now set after mode sequence being set.
Note that due to BSI (Back Side Illuminated) technology, image capture
is physically mirrored, mirror logic is so inversed in REG21 register
to cancel this effect.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
drivers/media/i2c/ov5640.c | 103 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 85 insertions(+), 18 deletions(-)
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index f6e40cc..41039e5 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -64,6 +64,7 @@
#define OV5640_REG_TIMING_DVPVO 0x380a
#define OV5640_REG_TIMING_HTS 0x380c
#define OV5640_REG_TIMING_VTS 0x380e
+#define OV5640_REG_TIMING_TC_REG20 0x3820
#define OV5640_REG_TIMING_TC_REG21 0x3821
#define OV5640_REG_AEC_CTRL00 0x3a00
#define OV5640_REG_AEC_B50_STEP 0x3a08
@@ -199,6 +200,8 @@ struct ov5640_ctrls {
struct v4l2_ctrl *contrast;
struct v4l2_ctrl *hue;
struct v4l2_ctrl *test_pattern;
+ struct v4l2_ctrl *hflip;
+ struct v4l2_ctrl *vflip;
};
struct ov5640_dev {
@@ -341,7 +344,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -360,7 +363,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -379,7 +382,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -399,7 +402,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -418,7 +421,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -437,7 +440,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -456,7 +459,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = {
{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -475,7 +478,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -494,7 +497,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_NTSC_720_480[] = {
{0x3035, 0x12, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -513,7 +516,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_NTSC_720_480[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -532,7 +535,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_30fps_PAL_720_576[] = {
{0x3035, 0x12, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -551,7 +554,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_PAL_720_576[] = {
{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
@@ -571,7 +574,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
{0x3008, 0x42, 0, 0},
{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x07, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
@@ -591,7 +594,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
static const struct reg_value ov5640_setting_15fps_720P_1280_720[] = {
{0x3035, 0x41, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x07, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
+ {0x3814, 0x31, 0, 0},
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
@@ -611,7 +614,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
{0x3008, 0x42, 0, 0},
{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
+ {0x3814, 0x11, 0, 0},
{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
@@ -644,7 +647,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
{0x3008, 0x42, 0, 0},
{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
+ {0x3814, 0x11, 0, 0},
{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
@@ -673,10 +676,9 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
};
static const struct reg_value ov5640_setting_15fps_QSXGA_2592_1944[] = {
- {0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0},
{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x08, 0, 0},
{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
- {0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
+ {0x3814, 0x11, 0, 0},
{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
@@ -1340,6 +1342,27 @@ static int ov5640_binning_on(struct ov5640_dev *sensor)
return temp ? 1 : 0;
}
+static int ov5640_set_binning(struct ov5640_dev *sensor, bool enable)
+{
+ int ret;
+
+ /*
+ * TIMING TC REG21:
+ * - [0]: Horizontal binning enable
+ */
+ ret = ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
+ BIT(0), enable ? BIT(0) : 0);
+ if (ret)
+ return ret;
+ /*
+ * TIMING TC REG20:
+ * - [0]: Undocumented, but hardcoded init sequences
+ * are always setting REG21/REG20 bit 0 to same value...
+ */
+ return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
+ BIT(0), enable ? BIT(0) : 0);
+}
+
static int ov5640_set_virtual_channel(struct ov5640_dev *sensor)
{
struct i2c_client *client = sensor->i2c_client;
@@ -1640,6 +1663,9 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
if (ret < 0)
return ret;
+ ret = ov5640_set_binning(sensor, dn_mode != SCALING);
+ if (ret < 0)
+ return ret;
ret = ov5640_set_ae_target(sensor, sensor->ae_target);
if (ret < 0)
return ret;
@@ -2193,6 +2219,37 @@ static int ov5640_set_ctrl_light_freq(struct ov5640_dev *sensor, int value)
BIT(2) : 0);
}
+static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
+{
+ /*
+ * Sensor is a BSI (Back Side Illuminated) one,
+ * so image captured is physically mirrored.
+ * This is why mirror logic is inversed in
+ * order to cancel this mirror effect.
+ */
+
+ /*
+ * TIMING TC REG21:
+ * - [2]: ISP mirror
+ * - [1]: Sensor mirror
+ */
+ return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
+ BIT(2) | BIT(1),
+ (!value) ? (BIT(2) | BIT(1)) : 0);
+}
+
+static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
+{
+ /*
+ * TIMING TC REG20:
+ * - [2]: ISP vflip
+ * - [1]: Sensor vflip
+ */
+ return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
+ BIT(2) | BIT(1),
+ value ? (BIT(2) | BIT(1)) : 0);
+}
+
static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
{
struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
@@ -2264,6 +2321,12 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_POWER_LINE_FREQUENCY:
ret = ov5640_set_ctrl_light_freq(sensor, ctrl->val);
break;
+ case V4L2_CID_HFLIP:
+ ret = ov5640_set_ctrl_hflip(sensor, ctrl->val);
+ break;
+ case V4L2_CID_VFLIP:
+ ret = ov5640_set_ctrl_vflip(sensor, ctrl->val);
+ break;
default:
ret = -EINVAL;
break;
@@ -2325,6 +2388,10 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
v4l2_ctrl_new_std_menu_items(hdl, ops, V4L2_CID_TEST_PATTERN,
ARRAY_SIZE(test_pattern_menu) - 1,
0, 0, test_pattern_menu);
+ ctrls->hflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP,
+ 0, 1, 1, 0);
+ ctrls->vflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP,
+ 0, 1, 1, 0);
ctrls->light_freq =
v4l2_ctrl_new_std_menu(hdl, ops,
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] media: ov5640: add support of module orientation
2018-06-11 9:29 ` Hugues Fruchet
@ 2018-06-11 9:29 ` Hugues Fruchet
-1 siblings, 0 replies; 17+ messages in thread
From: Hugues Fruchet @ 2018-06-11 9:29 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Hans Verkuil,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Rob Herring, Mark Rutland
Cc: devicetree, linux-media,
Hugues Fruchet <hugues.fruchet@st.com>,
Benjamin Gaignard, Maxime Ripard
Add support of module being physically mounted upside down.
In this case, mirror and flip are enabled to fix captured images
orientation.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
.../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
index 8e36da0..f76eb7e 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.
+- rotation: integer property; valid values are 0 (sensor mounted upright)
+ and 180 (sensor mounted upside down).
The device node must contain one 'port' child node for its digital output
video port, in accordance with the video interface bindings defined in
@@ -51,6 +53,7 @@ Examples:
DVDD-supply = <&vgen2_reg>; /* 1.5v */
powerdown-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
+ rotation = <180>;
port {
/* MIPI CSI-2 bus endpoint */
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 41039e5..5529b14 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -215,6 +215,7 @@ struct ov5640_dev {
struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
struct gpio_desc *reset_gpio;
struct gpio_desc *pwdn_gpio;
+ bool upside_down;
/* lock to protect all members below */
struct mutex lock;
@@ -2222,6 +2223,8 @@ static int ov5640_set_ctrl_light_freq(struct ov5640_dev *sensor, int value)
static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
{
/*
+ * If sensor is mounted upside down, mirror logic is inversed.
+ *
* Sensor is a BSI (Back Side Illuminated) one,
* so image captured is physically mirrored.
* This is why mirror logic is inversed in
@@ -2235,11 +2238,14 @@ static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
*/
return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
BIT(2) | BIT(1),
- (!value) ? (BIT(2) | BIT(1)) : 0);
+ (!(value ^ sensor->upside_down)) ?
+ (BIT(2) | BIT(1)) : 0);
}
static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
{
+ /* If sensor is mounted upside down, flip logic is inversed */
+
/*
* TIMING TC REG20:
* - [2]: ISP vflip
@@ -2247,7 +2253,8 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
*/
return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
BIT(2) | BIT(1),
- value ? (BIT(2) | BIT(1)) : 0);
+ (value ^ sensor->upside_down) ?
+ (BIT(2) | BIT(1)) : 0);
}
static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
@@ -2625,6 +2632,7 @@ static int ov5640_probe(struct i2c_client *client,
struct fwnode_handle *endpoint;
struct ov5640_dev *sensor;
struct v4l2_mbus_framefmt *fmt;
+ u32 rotation;
int ret;
sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
@@ -2650,6 +2658,22 @@ static int ov5640_probe(struct i2c_client *client,
sensor->ae_target = 52;
+ /* optional indication of physical rotation of sensor */
+ ret = fwnode_property_read_u32(of_fwnode_handle(client->dev.of_node),
+ "rotation", &rotation);
+ if (!ret) {
+ switch (rotation) {
+ case 180:
+ sensor->upside_down = true;
+ /* fall through */
+ case 0:
+ break;
+ default:
+ dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
+ rotation);
+ }
+ }
+
endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
NULL);
if (!endpoint) {
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] media: ov5640: add support of module orientation
@ 2018-06-11 9:29 ` Hugues Fruchet
0 siblings, 0 replies; 17+ messages in thread
From: Hugues Fruchet @ 2018-06-11 9:29 UTC (permalink / raw)
To: Steve Longerbeam, Sakari Ailus, Hans Verkuil,
Mauro Carvalho Chehab, Rob Herring, Mark Rutland
Cc: devicetree, linux-media, Hugues Fruchet, Benjamin Gaignard,
Maxime Ripard
Add support of module being physically mounted upside down.
In this case, mirror and flip are enabled to fix captured images
orientation.
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
.../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
index 8e36da0..f76eb7e 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.
+- rotation: integer property; valid values are 0 (sensor mounted upright)
+ and 180 (sensor mounted upside down).
The device node must contain one 'port' child node for its digital output
video port, in accordance with the video interface bindings defined in
@@ -51,6 +53,7 @@ Examples:
DVDD-supply = <&vgen2_reg>; /* 1.5v */
powerdown-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
+ rotation = <180>;
port {
/* MIPI CSI-2 bus endpoint */
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 41039e5..5529b14 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -215,6 +215,7 @@ struct ov5640_dev {
struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
struct gpio_desc *reset_gpio;
struct gpio_desc *pwdn_gpio;
+ bool upside_down;
/* lock to protect all members below */
struct mutex lock;
@@ -2222,6 +2223,8 @@ static int ov5640_set_ctrl_light_freq(struct ov5640_dev *sensor, int value)
static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
{
/*
+ * If sensor is mounted upside down, mirror logic is inversed.
+ *
* Sensor is a BSI (Back Side Illuminated) one,
* so image captured is physically mirrored.
* This is why mirror logic is inversed in
@@ -2235,11 +2238,14 @@ static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
*/
return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
BIT(2) | BIT(1),
- (!value) ? (BIT(2) | BIT(1)) : 0);
+ (!(value ^ sensor->upside_down)) ?
+ (BIT(2) | BIT(1)) : 0);
}
static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
{
+ /* If sensor is mounted upside down, flip logic is inversed */
+
/*
* TIMING TC REG20:
* - [2]: ISP vflip
@@ -2247,7 +2253,8 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
*/
return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
BIT(2) | BIT(1),
- value ? (BIT(2) | BIT(1)) : 0);
+ (value ^ sensor->upside_down) ?
+ (BIT(2) | BIT(1)) : 0);
}
static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
@@ -2625,6 +2632,7 @@ static int ov5640_probe(struct i2c_client *client,
struct fwnode_handle *endpoint;
struct ov5640_dev *sensor;
struct v4l2_mbus_framefmt *fmt;
+ u32 rotation;
int ret;
sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
@@ -2650,6 +2658,22 @@ static int ov5640_probe(struct i2c_client *client,
sensor->ae_target = 52;
+ /* optional indication of physical rotation of sensor */
+ ret = fwnode_property_read_u32(of_fwnode_handle(client->dev.of_node),
+ "rotation", &rotation);
+ if (!ret) {
+ switch (rotation) {
+ case 180:
+ sensor->upside_down = true;
+ /* fall through */
+ case 0:
+ break;
+ default:
+ dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
+ rotation);
+ }
+ }
+
endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
NULL);
if (!endpoint) {
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
2018-06-11 9:29 ` Hugues Fruchet
(?)
@ 2018-06-11 10:10 ` Sakari Ailus
2018-06-11 10:19 ` Hugues FRUCHET
-1 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2018-06-11 10:10 UTC (permalink / raw)
To: Hugues Fruchet
Cc: Steve Longerbeam, Hans Verkuil, Mauro Carvalho Chehab,
Rob Herring, Mark Rutland, devicetree, linux-media,
Benjamin Gaignard, Maxime Ripard
On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
> Add support of module being physically mounted upside down.
> In this case, mirror and flip are enabled to fix captured images
> orientation.
>
> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> ---
> .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
> drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> index 8e36da0..f76eb7e 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.
> +- rotation: integer property; valid values are 0 (sensor mounted upright)
> + and 180 (sensor mounted upside down).
>
> The device node must contain one 'port' child node for its digital output
> video port, in accordance with the video interface bindings defined in
> @@ -51,6 +53,7 @@ Examples:
> DVDD-supply = <&vgen2_reg>; /* 1.5v */
> powerdown-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
> reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
> + rotation = <180>;
>
> port {
> /* MIPI CSI-2 bus endpoint */
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 41039e5..5529b14 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -215,6 +215,7 @@ struct ov5640_dev {
> struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
> struct gpio_desc *reset_gpio;
> struct gpio_desc *pwdn_gpio;
> + bool upside_down;
>
> /* lock to protect all members below */
> struct mutex lock;
> @@ -2222,6 +2223,8 @@ static int ov5640_set_ctrl_light_freq(struct ov5640_dev *sensor, int value)
> static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
> {
> /*
> + * If sensor is mounted upside down, mirror logic is inversed.
> + *
> * Sensor is a BSI (Back Side Illuminated) one,
> * so image captured is physically mirrored.
> * This is why mirror logic is inversed in
> @@ -2235,11 +2238,14 @@ static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
> */
> return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
> BIT(2) | BIT(1),
> - (!value) ? (BIT(2) | BIT(1)) : 0);
> + (!(value ^ sensor->upside_down)) ?
> + (BIT(2) | BIT(1)) : 0);
> }
>
> static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
> {
> + /* If sensor is mounted upside down, flip logic is inversed */
> +
> /*
> * TIMING TC REG20:
> * - [2]: ISP vflip
> @@ -2247,7 +2253,8 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
> */
> return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
> BIT(2) | BIT(1),
> - value ? (BIT(2) | BIT(1)) : 0);
> + (value ^ sensor->upside_down) ?
> + (BIT(2) | BIT(1)) : 0);
> }
>
> static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> @@ -2625,6 +2632,7 @@ static int ov5640_probe(struct i2c_client *client,
> struct fwnode_handle *endpoint;
> struct ov5640_dev *sensor;
> struct v4l2_mbus_framefmt *fmt;
> + u32 rotation;
> int ret;
>
> sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
> @@ -2650,6 +2658,22 @@ static int ov5640_probe(struct i2c_client *client,
>
> sensor->ae_target = 52;
>
> + /* optional indication of physical rotation of sensor */
> + ret = fwnode_property_read_u32(of_fwnode_handle(client->dev.of_node),
Instead of of_fwnode_handle(), please use dev_fwnode(&client->dev) --- as the
driver already does elsewhere.
I can make the change if you're happy with that; the patches seem fine
otherwise.
> + "rotation", &rotation);
> + if (!ret) {
> + switch (rotation) {
> + case 180:
> + sensor->upside_down = true;
> + /* fall through */
> + case 0:
> + break;
> + default:
> + dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
> + rotation);
> + }
> + }
> +
> endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
> NULL);
> if (!endpoint) {
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
2018-06-11 10:10 ` Sakari Ailus
@ 2018-06-11 10:19 ` Hugues FRUCHET
0 siblings, 0 replies; 17+ messages in thread
From: Hugues FRUCHET @ 2018-06-11 10:19 UTC (permalink / raw)
To: Sakari Ailus
Cc: Steve Longerbeam, Hans Verkuil, Mauro Carvalho Chehab,
Rob Herring, Mark Rutland, devicetree, linux-media,
Benjamin Gaignard <benjamin.gaignard@linaro.org>,
Maxime Ripard
Hi Sakari,
I'm fine with the change to dev_fwnode(&client->dev).
Many thanks Sakari,
Hugues.
On 06/11/2018 12:10 PM, Sakari Ailus wrote:
> On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
>> Add support of module being physically mounted upside down.
>> In this case, mirror and flip are enabled to fix captured images
>> orientation.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
>> ---
>> .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
>> drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
>> 2 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>> index 8e36da0..f76eb7e 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.
>> +- rotation: integer property; valid values are 0 (sensor mounted upright)
>> + and 180 (sensor mounted upside down).
>>
>> The device node must contain one 'port' child node for its digital output
>> video port, in accordance with the video interface bindings defined in
>> @@ -51,6 +53,7 @@ Examples:
>> DVDD-supply = <&vgen2_reg>; /* 1.5v */
>> powerdown-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
>> reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
>> + rotation = <180>;
>>
>> port {
>> /* MIPI CSI-2 bus endpoint */
>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>> index 41039e5..5529b14 100644
>> --- a/drivers/media/i2c/ov5640.c
>> +++ b/drivers/media/i2c/ov5640.c
>> @@ -215,6 +215,7 @@ struct ov5640_dev {
>> struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
>> struct gpio_desc *reset_gpio;
>> struct gpio_desc *pwdn_gpio;
>> + bool upside_down;
>>
>> /* lock to protect all members below */
>> struct mutex lock;
>> @@ -2222,6 +2223,8 @@ static int ov5640_set_ctrl_light_freq(struct ov5640_dev *sensor, int value)
>> static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
>> {
>> /*
>> + * If sensor is mounted upside down, mirror logic is inversed.
>> + *
>> * Sensor is a BSI (Back Side Illuminated) one,
>> * so image captured is physically mirrored.
>> * This is why mirror logic is inversed in
>> @@ -2235,11 +2238,14 @@ static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
>> */
>> return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
>> BIT(2) | BIT(1),
>> - (!value) ? (BIT(2) | BIT(1)) : 0);
>> + (!(value ^ sensor->upside_down)) ?
>> + (BIT(2) | BIT(1)) : 0);
>> }
>>
>> static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
>> {
>> + /* If sensor is mounted upside down, flip logic is inversed */
>> +
>> /*
>> * TIMING TC REG20:
>> * - [2]: ISP vflip
>> @@ -2247,7 +2253,8 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
>> */
>> return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
>> BIT(2) | BIT(1),
>> - value ? (BIT(2) | BIT(1)) : 0);
>> + (value ^ sensor->upside_down) ?
>> + (BIT(2) | BIT(1)) : 0);
>> }
>>
>> static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>> @@ -2625,6 +2632,7 @@ static int ov5640_probe(struct i2c_client *client,
>> struct fwnode_handle *endpoint;
>> struct ov5640_dev *sensor;
>> struct v4l2_mbus_framefmt *fmt;
>> + u32 rotation;
>> int ret;
>>
>> sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
>> @@ -2650,6 +2658,22 @@ static int ov5640_probe(struct i2c_client *client,
>>
>> sensor->ae_target = 52;
>>
>> + /* optional indication of physical rotation of sensor */
>> + ret = fwnode_property_read_u32(of_fwnode_handle(client->dev.of_node),
>
> Instead of of_fwnode_handle(), please use dev_fwnode(&client->dev) --- as the
> driver already does elsewhere.
>
> I can make the change if you're happy with that; the patches seem fine
> otherwise.
>
>> + "rotation", &rotation);
>> + if (!ret) {
>> + switch (rotation) {
>> + case 180:
>> + sensor->upside_down = true;
>> + /* fall through */
>> + case 0:
>> + break;
>> + default:
>> + dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
>> + rotation);
>> + }
>> + }
>> +
>> endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
>> NULL);
>> if (!endpoint) {
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
@ 2018-06-11 10:19 ` Hugues FRUCHET
0 siblings, 0 replies; 17+ messages in thread
From: Hugues FRUCHET @ 2018-06-11 10:19 UTC (permalink / raw)
To: Sakari Ailus
Cc: Steve Longerbeam, Hans Verkuil, Mauro Carvalho Chehab,
Rob Herring, Mark Rutland, devicetree, linux-media,
Benjamin Gaignard, Maxime Ripard
Hi Sakari,
I'm fine with the change to dev_fwnode(&client->dev).
Many thanks Sakari,
Hugues.
On 06/11/2018 12:10 PM, Sakari Ailus wrote:
> On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
>> Add support of module being physically mounted upside down.
>> In this case, mirror and flip are enabled to fix captured images
>> orientation.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
>> ---
>> .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
>> drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
>> 2 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>> index 8e36da0..f76eb7e 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.
>> +- rotation: integer property; valid values are 0 (sensor mounted upright)
>> + and 180 (sensor mounted upside down).
>>
>> The device node must contain one 'port' child node for its digital output
>> video port, in accordance with the video interface bindings defined in
>> @@ -51,6 +53,7 @@ Examples:
>> DVDD-supply = <&vgen2_reg>; /* 1.5v */
>> powerdown-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
>> reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
>> + rotation = <180>;
>>
>> port {
>> /* MIPI CSI-2 bus endpoint */
>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>> index 41039e5..5529b14 100644
>> --- a/drivers/media/i2c/ov5640.c
>> +++ b/drivers/media/i2c/ov5640.c
>> @@ -215,6 +215,7 @@ struct ov5640_dev {
>> struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
>> struct gpio_desc *reset_gpio;
>> struct gpio_desc *pwdn_gpio;
>> + bool upside_down;
>>
>> /* lock to protect all members below */
>> struct mutex lock;
>> @@ -2222,6 +2223,8 @@ static int ov5640_set_ctrl_light_freq(struct ov5640_dev *sensor, int value)
>> static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
>> {
>> /*
>> + * If sensor is mounted upside down, mirror logic is inversed.
>> + *
>> * Sensor is a BSI (Back Side Illuminated) one,
>> * so image captured is physically mirrored.
>> * This is why mirror logic is inversed in
>> @@ -2235,11 +2238,14 @@ static int ov5640_set_ctrl_hflip(struct ov5640_dev *sensor, int value)
>> */
>> return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG21,
>> BIT(2) | BIT(1),
>> - (!value) ? (BIT(2) | BIT(1)) : 0);
>> + (!(value ^ sensor->upside_down)) ?
>> + (BIT(2) | BIT(1)) : 0);
>> }
>>
>> static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
>> {
>> + /* If sensor is mounted upside down, flip logic is inversed */
>> +
>> /*
>> * TIMING TC REG20:
>> * - [2]: ISP vflip
>> @@ -2247,7 +2253,8 @@ static int ov5640_set_ctrl_vflip(struct ov5640_dev *sensor, int value)
>> */
>> return ov5640_mod_reg(sensor, OV5640_REG_TIMING_TC_REG20,
>> BIT(2) | BIT(1),
>> - value ? (BIT(2) | BIT(1)) : 0);
>> + (value ^ sensor->upside_down) ?
>> + (BIT(2) | BIT(1)) : 0);
>> }
>>
>> static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
>> @@ -2625,6 +2632,7 @@ static int ov5640_probe(struct i2c_client *client,
>> struct fwnode_handle *endpoint;
>> struct ov5640_dev *sensor;
>> struct v4l2_mbus_framefmt *fmt;
>> + u32 rotation;
>> int ret;
>>
>> sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
>> @@ -2650,6 +2658,22 @@ static int ov5640_probe(struct i2c_client *client,
>>
>> sensor->ae_target = 52;
>>
>> + /* optional indication of physical rotation of sensor */
>> + ret = fwnode_property_read_u32(of_fwnode_handle(client->dev.of_node),
>
> Instead of of_fwnode_handle(), please use dev_fwnode(&client->dev) --- as the
> driver already does elsewhere.
>
> I can make the change if you're happy with that; the patches seem fine
> otherwise.
>
>> + "rotation", &rotation);
>> + if (!ret) {
>> + switch (rotation) {
>> + case 180:
>> + sensor->upside_down = true;
>> + /* fall through */
>> + case 0:
>> + break;
>> + default:
>> + dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
>> + rotation);
>> + }
>> + }
>> +
>> endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev),
>> NULL);
>> if (!endpoint) {
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
2018-06-11 9:29 ` Hugues Fruchet
(?)
(?)
@ 2018-06-12 22:06 ` Rob Herring
2018-06-13 7:43 ` Sakari Ailus
2018-06-13 8:10 ` Hugues FRUCHET
-1 siblings, 2 replies; 17+ messages in thread
From: Rob Herring @ 2018-06-12 22:06 UTC (permalink / raw)
To: Hugues Fruchet
Cc: Steve Longerbeam, Sakari Ailus, Hans Verkuil,
Mauro Carvalho Chehab, Mark Rutland, devicetree, linux-media,
Benjamin Gaignard, Maxime Ripard
On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
> Add support of module being physically mounted upside down.
> In this case, mirror and flip are enabled to fix captured images
> orientation.
>
> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> ---
> .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
Please split bindings to separate patches.
> drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> index 8e36da0..f76eb7e 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.
> +- rotation: integer property; valid values are 0 (sensor mounted upright)
> + and 180 (sensor mounted upside down).
Didn't we just add this as a common property? If so, just reference the
common definition. If not, it needs a common definition.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
2018-06-12 22:06 ` Rob Herring
@ 2018-06-13 7:43 ` Sakari Ailus
2018-06-13 8:10 ` Hugues FRUCHET
1 sibling, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2018-06-13 7:43 UTC (permalink / raw)
To: Rob Herring
Cc: Hugues Fruchet, Steve Longerbeam, Hans Verkuil,
Mauro Carvalho Chehab, Mark Rutland, devicetree, linux-media,
Benjamin Gaignard, Maxime Ripard
Hi Rob, Hugues,
On Tue, Jun 12, 2018 at 04:06:28PM -0600, Rob Herring wrote:
> On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
> > Add support of module being physically mounted upside down.
> > In this case, mirror and flip are enabled to fix captured images
> > orientation.
> >
> > Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> > ---
> > .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
>
> Please split bindings to separate patches.
>
> > drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
> > 2 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> > index 8e36da0..f76eb7e 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.
> > +- rotation: integer property; valid values are 0 (sensor mounted upright)
> > + and 180 (sensor mounted upside down).
>
> Didn't we just add this as a common property? If so, just reference the
> common definition. If not, it needs a common definition.
The common definition is there --- and this text is actually the same as
for the smiapp DT bindings --- which you acked. :-) I thought this would be
fine as well, and this patch is actually already in a pull request to
Mauro.
I put the smiapp bindings to the same patch with the driver change as they
were pretty small both but we'll split these in the future.
I've marked the pull request as deferred for now; let me know whether
you're still ok with this going in as such.
Thanks.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
2018-06-12 22:06 ` Rob Herring
@ 2018-06-13 8:10 ` Hugues FRUCHET
2018-06-13 8:10 ` Hugues FRUCHET
1 sibling, 0 replies; 17+ messages in thread
From: Hugues FRUCHET @ 2018-06-13 8:10 UTC (permalink / raw)
To: Rob Herring
Cc: Steve Longerbeam, Sakari Ailus, Hans Verkuil,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Mark Rutland, devicetree, linux-media,
Benjamin Gaignard <benjamin.gaignard@linaro.org>,
Maxime Ripard
Hi Rob, thanks for review,
On 06/13/2018 12:06 AM, Rob Herring wrote:
> On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
>> Add support of module being physically mounted upside down.
>> In this case, mirror and flip are enabled to fix captured images
>> orientation.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
>> ---
>> .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
>
> Please split bindings to separate patches.
OK, will do in next patchset.
>
>> drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
>> 2 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>> index 8e36da0..f76eb7e 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.
>> +- rotation: integer property; valid values are 0 (sensor mounted upright)
>> + and 180 (sensor mounted upside down).
>
> Didn't we just add this as a common property? If so, just reference the
> common definition. If not, it needs a common definition.
>
A common definition has been introduced by Sakari, I'm reusing it, see:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg132517.html
I would so propose:
>> +- rotation: as defined in
>> + Documentation/devicetree/bindings/media/video-interfaces.txt.
Best regards,
Hugues.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
@ 2018-06-13 8:10 ` Hugues FRUCHET
0 siblings, 0 replies; 17+ messages in thread
From: Hugues FRUCHET @ 2018-06-13 8:10 UTC (permalink / raw)
To: Rob Herring
Cc: Steve Longerbeam, Sakari Ailus, Hans Verkuil,
Mauro Carvalho Chehab, Mark Rutland, devicetree, linux-media,
Benjamin Gaignard, Maxime Ripard
Hi Rob, thanks for review,
On 06/13/2018 12:06 AM, Rob Herring wrote:
> On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
>> Add support of module being physically mounted upside down.
>> In this case, mirror and flip are enabled to fix captured images
>> orientation.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
>> ---
>> .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
>
> Please split bindings to separate patches.
OK, will do in next patchset.
>
>> drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
>> 2 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>> index 8e36da0..f76eb7e 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.
>> +- rotation: integer property; valid values are 0 (sensor mounted upright)
>> + and 180 (sensor mounted upside down).
>
> Didn't we just add this as a common property? If so, just reference the
> common definition. If not, it needs a common definition.
>
A common definition has been introduced by Sakari, I'm reusing it, see:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg132517.html
I would so propose:
>> +- rotation: as defined in
>> + Documentation/devicetree/bindings/media/video-interfaces.txt.
Best regards,
Hugues.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
2018-06-13 8:10 ` Hugues FRUCHET
(?)
@ 2018-06-13 8:24 ` Sakari Ailus
2018-06-13 10:09 ` Hugues FRUCHET
-1 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2018-06-13 8:24 UTC (permalink / raw)
To: Hugues FRUCHET
Cc: Rob Herring, Steve Longerbeam, Hans Verkuil,
Mauro Carvalho Chehab, Mark Rutland, devicetree, linux-media,
Benjamin Gaignard, Maxime Ripard
On Wed, Jun 13, 2018 at 08:10:02AM +0000, Hugues FRUCHET wrote:
> Hi Rob, thanks for review,
>
> On 06/13/2018 12:06 AM, Rob Herring wrote:
> > On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
> >> Add support of module being physically mounted upside down.
> >> In this case, mirror and flip are enabled to fix captured images
> >> orientation.
> >>
> >> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> >> ---
> >> .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
> >
> > Please split bindings to separate patches.
>
> OK, will do in next patchset.
>
> >
> >> drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
> >> 2 files changed, 29 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> >> index 8e36da0..f76eb7e 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.
> >> +- rotation: integer property; valid values are 0 (sensor mounted upright)
> >> + and 180 (sensor mounted upside down).
> >
> > Didn't we just add this as a common property? If so, just reference the
> > common definition. If not, it needs a common definition.
> >
>
> A common definition has been introduced by Sakari, I'm reusing it, see:
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg132517.html
>
> I would so propose:
> >> +- rotation: as defined in
> >> + Documentation/devicetree/bindings/media/video-interfaces.txt.
Shouldn't the description still include the valid values? As far as I can
tell, these are ultimately device specific albeit more or less the same for
*this kind* of sensors.
The file already contains a reference to video-interfaces.txt.
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
2018-06-13 8:24 ` Sakari Ailus
@ 2018-06-13 10:09 ` Hugues FRUCHET
0 siblings, 0 replies; 17+ messages in thread
From: Hugues FRUCHET @ 2018-06-13 10:09 UTC (permalink / raw)
To: Sakari Ailus
Cc: Rob Herring, Steve Longerbeam, Hans Verkuil,
Mauro Carvalho Chehab, Mark Rutland, devicetree, linux-media,
Benjamin Gaignard <benjamin.gaignard@linaro.org>,
Maxime Ripard
Hi Sakari, Rob,
Find a new proposal below:
On 06/13/2018 10:24 AM, Sakari Ailus wrote:
> On Wed, Jun 13, 2018 at 08:10:02AM +0000, Hugues FRUCHET wrote:
>> Hi Rob, thanks for review,
>>
>> On 06/13/2018 12:06 AM, Rob Herring wrote:
>>> On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
>>>> Add support of module being physically mounted upside down.
>>>> In this case, mirror and flip are enabled to fix captured images
>>>> orientation.
>>>>
>>>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
>>>> ---
>>>> .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
>>>
>>> Please split bindings to separate patches.
>>
>> OK, will do in next patchset.
>>
>>>
>>>> drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
>>>> 2 files changed, 29 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>>>> index 8e36da0..f76eb7e 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.
>>>> +- rotation: integer property; valid values are 0 (sensor mounted upright)
>>>> + and 180 (sensor mounted upside down).
>>>
>>> Didn't we just add this as a common property? If so, just reference the
>>> common definition. If not, it needs a common definition.
>>>
>>
>> A common definition has been introduced by Sakari, I'm reusing it, see:
>> https://www.mail-archive.com/linux-media@vger.kernel.org/msg132517.html
>>
>> I would so propose:
>> >> +- rotation: as defined in
>> >> + Documentation/devicetree/bindings/media/video-interfaces.txt.
>
> Shouldn't the description still include the valid values? As far as I can
> tell, these are ultimately device specific albeit more or less the same for
> *this kind* of sensors.
Yes you're right, let's put both together:
+- rotation: as defined in
+ Documentation/devicetree/bindings/media/video-interfaces.txt,
+ valid values are 0 (sensor mounted upright) and 180 (sensor
+ mounted upside down).
>
> The file already contains a reference to video-interfaces.txt.
>
Yes but it was related to 'port' child node.
Best regards,
Hugues.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
@ 2018-06-13 10:09 ` Hugues FRUCHET
0 siblings, 0 replies; 17+ messages in thread
From: Hugues FRUCHET @ 2018-06-13 10:09 UTC (permalink / raw)
To: Sakari Ailus
Cc: Rob Herring, Steve Longerbeam, Hans Verkuil,
Mauro Carvalho Chehab, Mark Rutland, devicetree, linux-media,
Benjamin Gaignard, Maxime Ripard
Hi Sakari, Rob,
Find a new proposal below:
On 06/13/2018 10:24 AM, Sakari Ailus wrote:
> On Wed, Jun 13, 2018 at 08:10:02AM +0000, Hugues FRUCHET wrote:
>> Hi Rob, thanks for review,
>>
>> On 06/13/2018 12:06 AM, Rob Herring wrote:
>>> On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
>>>> Add support of module being physically mounted upside down.
>>>> In this case, mirror and flip are enabled to fix captured images
>>>> orientation.
>>>>
>>>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
>>>> ---
>>>> .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
>>>
>>> Please split bindings to separate patches.
>>
>> OK, will do in next patchset.
>>
>>>
>>>> drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
>>>> 2 files changed, 29 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
>>>> index 8e36da0..f76eb7e 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.
>>>> +- rotation: integer property; valid values are 0 (sensor mounted upright)
>>>> + and 180 (sensor mounted upside down).
>>>
>>> Didn't we just add this as a common property? If so, just reference the
>>> common definition. If not, it needs a common definition.
>>>
>>
>> A common definition has been introduced by Sakari, I'm reusing it, see:
>> https://www.mail-archive.com/linux-media@vger.kernel.org/msg132517.html
>>
>> I would so propose:
>> >> +- rotation: as defined in
>> >> + Documentation/devicetree/bindings/media/video-interfaces.txt.
>
> Shouldn't the description still include the valid values? As far as I can
> tell, these are ultimately device specific albeit more or less the same for
> *this kind* of sensors.
Yes you're right, let's put both together:
+- rotation: as defined in
+ Documentation/devicetree/bindings/media/video-interfaces.txt,
+ valid values are 0 (sensor mounted upright) and 180 (sensor
+ mounted upside down).
>
> The file already contains a reference to video-interfaces.txt.
>
Yes but it was related to 'port' child node.
Best regards,
Hugues.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ov5640: add support of module orientation
2018-06-13 10:09 ` Hugues FRUCHET
(?)
@ 2018-06-13 12:11 ` Sakari Ailus
-1 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2018-06-13 12:11 UTC (permalink / raw)
To: Hugues FRUCHET
Cc: Rob Herring, Steve Longerbeam, Hans Verkuil,
Mauro Carvalho Chehab, Mark Rutland, devicetree, linux-media,
Benjamin Gaignard, Maxime Ripard
On Wed, Jun 13, 2018 at 10:09:58AM +0000, Hugues FRUCHET wrote:
> Hi Sakari, Rob,
>
> Find a new proposal below:
>
> On 06/13/2018 10:24 AM, Sakari Ailus wrote:
> > On Wed, Jun 13, 2018 at 08:10:02AM +0000, Hugues FRUCHET wrote:
> >> Hi Rob, thanks for review,
> >>
> >> On 06/13/2018 12:06 AM, Rob Herring wrote:
> >>> On Mon, Jun 11, 2018 at 11:29:17AM +0200, Hugues Fruchet wrote:
> >>>> Add support of module being physically mounted upside down.
> >>>> In this case, mirror and flip are enabled to fix captured images
> >>>> orientation.
> >>>>
> >>>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> >>>> ---
> >>>> .../devicetree/bindings/media/i2c/ov5640.txt | 3 +++
> >>>
> >>> Please split bindings to separate patches.
> >>
> >> OK, will do in next patchset.
> >>
> >>>
> >>>> drivers/media/i2c/ov5640.c | 28 ++++++++++++++++++++--
> >>>> 2 files changed, 29 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5640.txt b/Documentation/devicetree/bindings/media/i2c/ov5640.txt
> >>>> index 8e36da0..f76eb7e 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.
> >>>> +- rotation: integer property; valid values are 0 (sensor mounted upright)
> >>>> + and 180 (sensor mounted upside down).
> >>>
> >>> Didn't we just add this as a common property? If so, just reference the
> >>> common definition. If not, it needs a common definition.
> >>>
> >>
> >> A common definition has been introduced by Sakari, I'm reusing it, see:
> >> https://www.mail-archive.com/linux-media@vger.kernel.org/msg132517.html
> >>
> >> I would so propose:
> >> >> +- rotation: as defined in
> >> >> + Documentation/devicetree/bindings/media/video-interfaces.txt.
> >
> > Shouldn't the description still include the valid values? As far as I can
> > tell, these are ultimately device specific albeit more or less the same for
> > *this kind* of sensors.
>
> Yes you're right, let's put both together:
> +- rotation: as defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt,
> + valid values are 0 (sensor mounted upright) and 180 (sensor
> + mounted upside down).
I'll improve the description for smiapp as well.
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-06-13 12:11 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-11 9:29 [PATCH 0/2] OV5640 hflip, vflip and module orientation support Hugues Fruchet
2018-06-11 9:29 ` Hugues Fruchet
2018-06-11 9:29 ` [PATCH 1/2] media: ov5640: add HFLIP/VFLIP controls support Hugues Fruchet
2018-06-11 9:29 ` Hugues Fruchet
2018-06-11 9:29 ` [PATCH 2/2] media: ov5640: add support of module orientation Hugues Fruchet
2018-06-11 9:29 ` Hugues Fruchet
2018-06-11 10:10 ` Sakari Ailus
2018-06-11 10:19 ` Hugues FRUCHET
2018-06-11 10:19 ` Hugues FRUCHET
2018-06-12 22:06 ` Rob Herring
2018-06-13 7:43 ` Sakari Ailus
2018-06-13 8:10 ` Hugues FRUCHET
2018-06-13 8:10 ` Hugues FRUCHET
2018-06-13 8:24 ` Sakari Ailus
2018-06-13 10:09 ` Hugues FRUCHET
2018-06-13 10:09 ` Hugues FRUCHET
2018-06-13 12:11 ` 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.