Hi Sakari
On Tue, Mar 28, 2023 at 03:16:24PM +0300, Sakari Ailus wrote:
> No longer mirror flipping controls if the sensor is mounted upside down.
> Instead, change the default flip control values.
>
> This way the behaviour of the flipping controls and rotation of the sensor
> are aligned with the rest of the drivers.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/ccs/ccs-core.c | 29 ++++-------------------------
> drivers/media/i2c/ccs/ccs.h | 1 -
> 2 files changed, 4 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
> index 4a14d7e5d9f2..431dd7d24cdc 100644
> --- a/drivers/media/i2c/ccs/ccs-core.c
> +++ b/drivers/media/i2c/ccs/ccs-core.c
> @@ -569,8 +569,6 @@ static u32 ccs_pixel_order(struct ccs_sensor *sensor)
> flip |= CCS_IMAGE_ORIENTATION_VERTICAL_FLIP;
> }
>
> - flip ^= sensor->hvflip_inv_mask;
> -
> dev_dbg(&client->dev, "flip %u\n", flip);
> return sensor->default_pixel_order ^ flip;
> }
> @@ -632,8 +630,6 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl)
> if (sensor->vflip->val)
> orient |= CCS_IMAGE_ORIENTATION_VERTICAL_FLIP;
>
> - orient ^= sensor->hvflip_inv_mask;
> -
> ccs_update_mbus_formats(sensor);
>
> break;
> @@ -800,6 +796,8 @@ static const struct v4l2_ctrl_ops ccs_ctrl_ops = {
> static int ccs_init_controls(struct ccs_sensor *sensor)
> {
> struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
> + int default_flip sensor->hwcfg.module_board_orient ==
I think there's something wrong here :)
> + CCS_MODULE_BOARD_ORIENT_180;
I would also be tempted to suggest dropping module_board_orient
completely and store the rotation value directly. But then you would
have to compare it to "180", "0" etc etc. Maybe it's not a good idea.
Or maybe instead of removing hvflip_inv_mask you could keep it and
remove module_board_orient instead, so that v and h flips can be
considered in isolation.
The rest of the patch looks good to me
Thanks
j
> int rval;
>
> rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 17);
> @@ -948,10 +946,10 @@ static int ccs_init_controls(struct ccs_sensor *sensor)
>
> sensor->hflip = v4l2_ctrl_new_std(
> &sensor->pixel_array->ctrl_handler, &ccs_ctrl_ops,
> - V4L2_CID_HFLIP, 0, 1, 1, 0);
> + V4L2_CID_HFLIP, 0, 1, 1, default_flip);
> sensor->vflip = v4l2_ctrl_new_std(
> &sensor->pixel_array->ctrl_handler, &ccs_ctrl_ops,
> - V4L2_CID_VFLIP, 0, 1, 1, 0);
> + V4L2_CID_VFLIP, 0, 1, 1, default_flip);
>
> sensor->vblank = v4l2_ctrl_new_std(
> &sensor->pixel_array->ctrl_handler, &ccs_ctrl_ops,
> @@ -3437,25 +3435,6 @@ static int ccs_probe(struct i2c_client *client)
> if (rval < 0)
> goto out_free_ccs_limits;
>
> - /*
> - * Handle Sensor Module orientation on the board.
> - *
> - * The application of H-FLIP and V-FLIP on the sensor is modified by
> - * the sensor orientation on the board.
> - *
> - * For CCS_BOARD_SENSOR_ORIENT_180 the default behaviour is to set
> - * both H-FLIP and V-FLIP for normal operation which also implies
> - * that a set/unset operation for user space HFLIP and VFLIP v4l2
> - * controls will need to be internally inverted.
> - *
> - * Rotation also changes the bayer pattern.
> - */
> - if (sensor->hwcfg.module_board_orient ==
> - CCS_MODULE_BOARD_ORIENT_180)
> - sensor->hvflip_inv_mask =
> - CCS_IMAGE_ORIENTATION_HORIZONTAL_MIRROR |
> - CCS_IMAGE_ORIENTATION_VERTICAL_FLIP;
> -
> rval = ccs_call_quirk(sensor, limits);
> if (rval) {
> dev_err(&client->dev, "limits quirks failed\n");
> diff --git a/drivers/media/i2c/ccs/ccs.h b/drivers/media/i2c/ccs/ccs.h
> index 6beac375cc48..ea8909f011d9 100644
> --- a/drivers/media/i2c/ccs/ccs.h
> +++ b/drivers/media/i2c/ccs/ccs.h
> @@ -243,7 +243,6 @@ struct ccs_sensor {
> u8 scale_m;
> u8 scaling_mode;
>
> - u8 hvflip_inv_mask; /* H/VFLIP inversion due to sensor orientation */
> u8 frame_skip;
> u16 embedded_start; /* embedded data start line */
> u16 embedded_end;
> --
> 2.30.2
>
