| Message ID | 20240710044633.81372-3-umang.jain@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: imx335: Support vertical flip | expand |
Hi Umang, On Wed, Jul 10, 2024 at 10:16:32AM +0530, Umang Jain wrote: > Support vertical flip by setting REG_VREVERSE. > Additional registers also needs to be set per mode, according > to the readout direction (normal/inverted) as mentioned in the > data sheet. > > Since the register IMX335_REG_AREA3_ST_ADR_1 is based on the > flip (and is set via vflip related registers), it has been > moved out of the 2592x1944 mode regs. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > drivers/media/i2c/imx335.c | 71 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 69 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c > index 6c1e61b6696b..cd150606a8a9 100644 > --- a/drivers/media/i2c/imx335.c > +++ b/drivers/media/i2c/imx335.c > @@ -56,6 +56,9 @@ > #define IMX335_AGAIN_STEP 1 > #define IMX335_AGAIN_DEFAULT 0 > > +/* Vertical flip */ > +#define IMX335_REG_VREVERSE CCI_REG8(0x304f) > + > #define IMX335_REG_TPG_TESTCLKEN CCI_REG8(0x3148) > > #define IMX335_REG_INCLKSEL1 CCI_REG16_LE(0x314c) > @@ -155,6 +158,8 @@ static const char * const imx335_supply_name[] = { > * @vblank_max: Maximum vertical blanking in lines > * @pclk: Sensor pixel clock > * @reg_list: Register list for sensor mode > + * @vflip_normal: Register list vflip (normal readout) > + * @vflip_inverted: Register list vflip (inverted readout) > */ > struct imx335_mode { > u32 width; > @@ -166,6 +171,8 @@ struct imx335_mode { > u32 vblank_max; > u64 pclk; > struct imx335_reg_list reg_list; > + struct imx335_reg_list vflip_normal; > + struct imx335_reg_list vflip_inverted; > }; > > /** > @@ -183,6 +190,7 @@ struct imx335_mode { > * @pclk_ctrl: Pointer to pixel clock control > * @hblank_ctrl: Pointer to horizontal blanking control > * @vblank_ctrl: Pointer to vertical blanking control > + * @vflip: Pointer to vertical flip control > * @exp_ctrl: Pointer to exposure control > * @again_ctrl: Pointer to analog gain control > * @vblank: Vertical blanking in lines > @@ -207,6 +215,7 @@ struct imx335 { > struct v4l2_ctrl *pclk_ctrl; > struct v4l2_ctrl *hblank_ctrl; > struct v4l2_ctrl *vblank_ctrl; > + struct v4l2_ctrl *vflip; > struct { > struct v4l2_ctrl *exp_ctrl; > struct v4l2_ctrl *again_ctrl; > @@ -259,7 +268,6 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = { > { IMX335_REG_HTRIMMING_START, 48 }, > { IMX335_REG_HNUM, 2592 }, > { IMX335_REG_Y_OUT_SIZE, 1944 }, > - { IMX335_REG_AREA3_ST_ADR_1, 176 }, > { IMX335_REG_AREA3_WIDTH_1, 3928 }, > { IMX335_REG_OPB_SIZE_V, 0 }, > { IMX335_REG_XVS_XHS_DRV, 0x00 }, > @@ -333,6 +341,26 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = { > { CCI_REG8(0x3a00), 0x00 }, > }; > > +static const struct cci_reg_sequence mode_2592x1944_vflip_normal[] = { > + { IMX335_REG_AREA3_ST_ADR_1, 176 }, > + > + /* Undocumented V-Flip related registers on Page 55 of datasheet. */ > + { CCI_REG8(0x3081), 0x02, }, > + { CCI_REG8(0x3083), 0x02, }, > + { CCI_REG16_LE(0x30b6), 0x00 }, > + { CCI_REG16_LE(0x3116), 0x08 }, > +}; > + > +static const struct cci_reg_sequence mode_2592x1944_vflip_inverted[] = { > + { IMX335_REG_AREA3_ST_ADR_1, 4112 }, > + > + /* Undocumented V-Flip related registers on Page 55 of datasheet. */ > + { CCI_REG8(0x3081), 0xfe, }, > + { CCI_REG8(0x3083), 0xfe, }, > + { CCI_REG16_LE(0x30b6), 0x1fa }, > + { CCI_REG16_LE(0x3116), 0x002 }, > +}; > + > static const struct cci_reg_sequence raw10_framefmt_regs[] = { > { IMX335_REG_ADBIT, 0x00 }, > { IMX335_REG_MDBIT, 0x00 }, > @@ -419,6 +447,14 @@ static const struct imx335_mode supported_mode = { > .num_of_regs = ARRAY_SIZE(mode_2592x1944_regs), > .regs = mode_2592x1944_regs, > }, > + .vflip_normal = { > + .num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_normal), > + .regs = mode_2592x1944_vflip_normal, > + }, > + .vflip_inverted = { > + .num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_inverted), > + .regs = mode_2592x1944_vflip_inverted, > + }, > }; > > /** > @@ -492,6 +528,26 @@ static int imx335_update_exp_gain(struct imx335 *imx335, u32 exposure, u32 gain) > return ret; > } > > +static int imx335_update_vertical_flip(struct imx335 *imx335, u32 vflip) > +{ > + int ret = 0; > + > + if (vflip) > + cci_multi_reg_write(imx335->cci, > + imx335->cur_mode->vflip_inverted.regs, > + imx335->cur_mode->vflip_inverted.num_of_regs, > + &ret); > + else > + cci_multi_reg_write(imx335->cci, > + imx335->cur_mode->vflip_normal.regs, > + imx335->cur_mode->vflip_normal.num_of_regs, > + &ret); > + if (ret) > + return ret; > + > + return cci_write(imx335->cci, IMX335_REG_VREVERSE, vflip, NULL); > +} > + > static int imx335_update_test_pattern(struct imx335 *imx335, u32 pattern_index) > { > int ret = 0; > @@ -584,6 +640,10 @@ static int imx335_set_ctrl(struct v4l2_ctrl *ctrl) > > ret = imx335_update_exp_gain(imx335, exposure, analog_gain); > > + break; > + case V4L2_CID_VFLIP: > + ret = imx335_update_vertical_flip(imx335, ctrl->val); > + > break; > case V4L2_CID_TEST_PATTERN: > ret = imx335_update_test_pattern(imx335, ctrl->val); > @@ -1167,7 +1227,7 @@ static int imx335_init_controls(struct imx335 *imx335) > return ret; > > /* v4l2_fwnode_device_properties can add two more controls */ > - ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9); > + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 10); > if (ret) > return ret; > > @@ -1202,6 +1262,13 @@ static int imx335_init_controls(struct imx335 *imx335) > > v4l2_ctrl_cluster(2, &imx335->exp_ctrl); > > + imx335->vflip = v4l2_ctrl_new_std(ctrl_hdlr, > + &imx335_ctrl_ops, > + V4L2_CID_VFLIP, > + 0, 1, 1, 0); > + if (imx335->vflip) > + imx335->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; > + > imx335->vblank_ctrl = v4l2_ctrl_new_std(ctrl_hdlr, > &imx335_ctrl_ops, > V4L2_CID_VBLANK, > -- > 2.45.0 > This patch looks good to me. Reviewed-by: Tommaso Merciai <tomm.merciai@gmail.com> Thanks & Regards, Tommaso
Quoting Umang Jain (2024-07-10 05:46:32) > Support vertical flip by setting REG_VREVERSE. > Additional registers also needs to be set per mode, according > to the readout direction (normal/inverted) as mentioned in the > data sheet. > > Since the register IMX335_REG_AREA3_ST_ADR_1 is based on the > flip (and is set via vflip related registers), it has been > moved out of the 2592x1944 mode regs. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > drivers/media/i2c/imx335.c | 71 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 69 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c > index 6c1e61b6696b..cd150606a8a9 100644 > --- a/drivers/media/i2c/imx335.c > +++ b/drivers/media/i2c/imx335.c > @@ -56,6 +56,9 @@ > #define IMX335_AGAIN_STEP 1 > #define IMX335_AGAIN_DEFAULT 0 > > +/* Vertical flip */ > +#define IMX335_REG_VREVERSE CCI_REG8(0x304f) > + > #define IMX335_REG_TPG_TESTCLKEN CCI_REG8(0x3148) > > #define IMX335_REG_INCLKSEL1 CCI_REG16_LE(0x314c) > @@ -155,6 +158,8 @@ static const char * const imx335_supply_name[] = { > * @vblank_max: Maximum vertical blanking in lines > * @pclk: Sensor pixel clock > * @reg_list: Register list for sensor mode > + * @vflip_normal: Register list vflip (normal readout) > + * @vflip_inverted: Register list vflip (inverted readout) > */ > struct imx335_mode { > u32 width; > @@ -166,6 +171,8 @@ struct imx335_mode { > u32 vblank_max; > u64 pclk; > struct imx335_reg_list reg_list; > + struct imx335_reg_list vflip_normal; > + struct imx335_reg_list vflip_inverted; > }; > > /** > @@ -183,6 +190,7 @@ struct imx335_mode { > * @pclk_ctrl: Pointer to pixel clock control > * @hblank_ctrl: Pointer to horizontal blanking control > * @vblank_ctrl: Pointer to vertical blanking control > + * @vflip: Pointer to vertical flip control > * @exp_ctrl: Pointer to exposure control > * @again_ctrl: Pointer to analog gain control > * @vblank: Vertical blanking in lines > @@ -207,6 +215,7 @@ struct imx335 { > struct v4l2_ctrl *pclk_ctrl; > struct v4l2_ctrl *hblank_ctrl; > struct v4l2_ctrl *vblank_ctrl; > + struct v4l2_ctrl *vflip; > struct { > struct v4l2_ctrl *exp_ctrl; > struct v4l2_ctrl *again_ctrl; > @@ -259,7 +268,6 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = { > { IMX335_REG_HTRIMMING_START, 48 }, > { IMX335_REG_HNUM, 2592 }, > { IMX335_REG_Y_OUT_SIZE, 1944 }, > - { IMX335_REG_AREA3_ST_ADR_1, 176 }, > { IMX335_REG_AREA3_WIDTH_1, 3928 }, > { IMX335_REG_OPB_SIZE_V, 0 }, > { IMX335_REG_XVS_XHS_DRV, 0x00 }, > @@ -333,6 +341,26 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = { > { CCI_REG8(0x3a00), 0x00 }, > }; > > +static const struct cci_reg_sequence mode_2592x1944_vflip_normal[] = { > + { IMX335_REG_AREA3_ST_ADR_1, 176 }, > + > + /* Undocumented V-Flip related registers on Page 55 of datasheet. */ > + { CCI_REG8(0x3081), 0x02, }, > + { CCI_REG8(0x3083), 0x02, }, > + { CCI_REG16_LE(0x30b6), 0x00 }, > + { CCI_REG16_LE(0x3116), 0x08 }, > +}; > + > +static const struct cci_reg_sequence mode_2592x1944_vflip_inverted[] = { > + { IMX335_REG_AREA3_ST_ADR_1, 4112 }, > + > + /* Undocumented V-Flip related registers on Page 55 of datasheet. */ > + { CCI_REG8(0x3081), 0xfe, }, > + { CCI_REG8(0x3083), 0xfe, }, > + { CCI_REG16_LE(0x30b6), 0x1fa }, > + { CCI_REG16_LE(0x3116), 0x002 }, A little more awkward than the usual flip controls, but I think we do need to track what the datasheet gives us for now unless we can get more information from Sony or do some reverse engineering here which isn't really worth the effort at the moment. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > +}; > + > static const struct cci_reg_sequence raw10_framefmt_regs[] = { > { IMX335_REG_ADBIT, 0x00 }, > { IMX335_REG_MDBIT, 0x00 }, > @@ -419,6 +447,14 @@ static const struct imx335_mode supported_mode = { > .num_of_regs = ARRAY_SIZE(mode_2592x1944_regs), > .regs = mode_2592x1944_regs, > }, > + .vflip_normal = { > + .num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_normal), > + .regs = mode_2592x1944_vflip_normal, > + }, > + .vflip_inverted = { > + .num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_inverted), > + .regs = mode_2592x1944_vflip_inverted, > + }, > }; > > /** > @@ -492,6 +528,26 @@ static int imx335_update_exp_gain(struct imx335 *imx335, u32 exposure, u32 gain) > return ret; > } > > +static int imx335_update_vertical_flip(struct imx335 *imx335, u32 vflip) > +{ > + int ret = 0; > + > + if (vflip) > + cci_multi_reg_write(imx335->cci, > + imx335->cur_mode->vflip_inverted.regs, > + imx335->cur_mode->vflip_inverted.num_of_regs, > + &ret); > + else > + cci_multi_reg_write(imx335->cci, > + imx335->cur_mode->vflip_normal.regs, > + imx335->cur_mode->vflip_normal.num_of_regs, > + &ret); > + if (ret) > + return ret; > + > + return cci_write(imx335->cci, IMX335_REG_VREVERSE, vflip, NULL); > +} > + > static int imx335_update_test_pattern(struct imx335 *imx335, u32 pattern_index) > { > int ret = 0; > @@ -584,6 +640,10 @@ static int imx335_set_ctrl(struct v4l2_ctrl *ctrl) > > ret = imx335_update_exp_gain(imx335, exposure, analog_gain); > > + break; > + case V4L2_CID_VFLIP: > + ret = imx335_update_vertical_flip(imx335, ctrl->val); > + > break; > case V4L2_CID_TEST_PATTERN: > ret = imx335_update_test_pattern(imx335, ctrl->val); > @@ -1167,7 +1227,7 @@ static int imx335_init_controls(struct imx335 *imx335) > return ret; > > /* v4l2_fwnode_device_properties can add two more controls */ > - ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9); > + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 10); > if (ret) > return ret; > > @@ -1202,6 +1262,13 @@ static int imx335_init_controls(struct imx335 *imx335) > > v4l2_ctrl_cluster(2, &imx335->exp_ctrl); > > + imx335->vflip = v4l2_ctrl_new_std(ctrl_hdlr, > + &imx335_ctrl_ops, > + V4L2_CID_VFLIP, > + 0, 1, 1, 0); > + if (imx335->vflip) > + imx335->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; > + > imx335->vblank_ctrl = v4l2_ctrl_new_std(ctrl_hdlr, > &imx335_ctrl_ops, > V4L2_CID_VBLANK, > -- > 2.45.0 >
diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c index 6c1e61b6696b..cd150606a8a9 100644 --- a/drivers/media/i2c/imx335.c +++ b/drivers/media/i2c/imx335.c @@ -56,6 +56,9 @@ #define IMX335_AGAIN_STEP 1 #define IMX335_AGAIN_DEFAULT 0 +/* Vertical flip */ +#define IMX335_REG_VREVERSE CCI_REG8(0x304f) + #define IMX335_REG_TPG_TESTCLKEN CCI_REG8(0x3148) #define IMX335_REG_INCLKSEL1 CCI_REG16_LE(0x314c) @@ -155,6 +158,8 @@ static const char * const imx335_supply_name[] = { * @vblank_max: Maximum vertical blanking in lines * @pclk: Sensor pixel clock * @reg_list: Register list for sensor mode + * @vflip_normal: Register list vflip (normal readout) + * @vflip_inverted: Register list vflip (inverted readout) */ struct imx335_mode { u32 width; @@ -166,6 +171,8 @@ struct imx335_mode { u32 vblank_max; u64 pclk; struct imx335_reg_list reg_list; + struct imx335_reg_list vflip_normal; + struct imx335_reg_list vflip_inverted; }; /** @@ -183,6 +190,7 @@ struct imx335_mode { * @pclk_ctrl: Pointer to pixel clock control * @hblank_ctrl: Pointer to horizontal blanking control * @vblank_ctrl: Pointer to vertical blanking control + * @vflip: Pointer to vertical flip control * @exp_ctrl: Pointer to exposure control * @again_ctrl: Pointer to analog gain control * @vblank: Vertical blanking in lines @@ -207,6 +215,7 @@ struct imx335 { struct v4l2_ctrl *pclk_ctrl; struct v4l2_ctrl *hblank_ctrl; struct v4l2_ctrl *vblank_ctrl; + struct v4l2_ctrl *vflip; struct { struct v4l2_ctrl *exp_ctrl; struct v4l2_ctrl *again_ctrl; @@ -259,7 +268,6 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = { { IMX335_REG_HTRIMMING_START, 48 }, { IMX335_REG_HNUM, 2592 }, { IMX335_REG_Y_OUT_SIZE, 1944 }, - { IMX335_REG_AREA3_ST_ADR_1, 176 }, { IMX335_REG_AREA3_WIDTH_1, 3928 }, { IMX335_REG_OPB_SIZE_V, 0 }, { IMX335_REG_XVS_XHS_DRV, 0x00 }, @@ -333,6 +341,26 @@ static const struct cci_reg_sequence mode_2592x1944_regs[] = { { CCI_REG8(0x3a00), 0x00 }, }; +static const struct cci_reg_sequence mode_2592x1944_vflip_normal[] = { + { IMX335_REG_AREA3_ST_ADR_1, 176 }, + + /* Undocumented V-Flip related registers on Page 55 of datasheet. */ + { CCI_REG8(0x3081), 0x02, }, + { CCI_REG8(0x3083), 0x02, }, + { CCI_REG16_LE(0x30b6), 0x00 }, + { CCI_REG16_LE(0x3116), 0x08 }, +}; + +static const struct cci_reg_sequence mode_2592x1944_vflip_inverted[] = { + { IMX335_REG_AREA3_ST_ADR_1, 4112 }, + + /* Undocumented V-Flip related registers on Page 55 of datasheet. */ + { CCI_REG8(0x3081), 0xfe, }, + { CCI_REG8(0x3083), 0xfe, }, + { CCI_REG16_LE(0x30b6), 0x1fa }, + { CCI_REG16_LE(0x3116), 0x002 }, +}; + static const struct cci_reg_sequence raw10_framefmt_regs[] = { { IMX335_REG_ADBIT, 0x00 }, { IMX335_REG_MDBIT, 0x00 }, @@ -419,6 +447,14 @@ static const struct imx335_mode supported_mode = { .num_of_regs = ARRAY_SIZE(mode_2592x1944_regs), .regs = mode_2592x1944_regs, }, + .vflip_normal = { + .num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_normal), + .regs = mode_2592x1944_vflip_normal, + }, + .vflip_inverted = { + .num_of_regs = ARRAY_SIZE(mode_2592x1944_vflip_inverted), + .regs = mode_2592x1944_vflip_inverted, + }, }; /** @@ -492,6 +528,26 @@ static int imx335_update_exp_gain(struct imx335 *imx335, u32 exposure, u32 gain) return ret; } +static int imx335_update_vertical_flip(struct imx335 *imx335, u32 vflip) +{ + int ret = 0; + + if (vflip) + cci_multi_reg_write(imx335->cci, + imx335->cur_mode->vflip_inverted.regs, + imx335->cur_mode->vflip_inverted.num_of_regs, + &ret); + else + cci_multi_reg_write(imx335->cci, + imx335->cur_mode->vflip_normal.regs, + imx335->cur_mode->vflip_normal.num_of_regs, + &ret); + if (ret) + return ret; + + return cci_write(imx335->cci, IMX335_REG_VREVERSE, vflip, NULL); +} + static int imx335_update_test_pattern(struct imx335 *imx335, u32 pattern_index) { int ret = 0; @@ -584,6 +640,10 @@ static int imx335_set_ctrl(struct v4l2_ctrl *ctrl) ret = imx335_update_exp_gain(imx335, exposure, analog_gain); + break; + case V4L2_CID_VFLIP: + ret = imx335_update_vertical_flip(imx335, ctrl->val); + break; case V4L2_CID_TEST_PATTERN: ret = imx335_update_test_pattern(imx335, ctrl->val); @@ -1167,7 +1227,7 @@ static int imx335_init_controls(struct imx335 *imx335) return ret; /* v4l2_fwnode_device_properties can add two more controls */ - ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9); + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 10); if (ret) return ret; @@ -1202,6 +1262,13 @@ static int imx335_init_controls(struct imx335 *imx335) v4l2_ctrl_cluster(2, &imx335->exp_ctrl); + imx335->vflip = v4l2_ctrl_new_std(ctrl_hdlr, + &imx335_ctrl_ops, + V4L2_CID_VFLIP, + 0, 1, 1, 0); + if (imx335->vflip) + imx335->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; + imx335->vblank_ctrl = v4l2_ctrl_new_std(ctrl_hdlr, &imx335_ctrl_ops, V4L2_CID_VBLANK,
Support vertical flip by setting REG_VREVERSE. Additional registers also needs to be set per mode, according to the readout direction (normal/inverted) as mentioned in the data sheet. Since the register IMX335_REG_AREA3_ST_ADR_1 is based on the flip (and is set via vflip related registers), it has been moved out of the 2592x1944 mode regs. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- drivers/media/i2c/imx335.c | 71 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 2 deletions(-)