[v4,8/8] media: imx274: add SELECTION support for cropping
Commit Message
Currently this driver does not support cropping. The supported modes
are the following, all capturing the entire area:
- 3840x2160, 1:1 binning (native sensor resolution)
- 1920x1080, 2:1 binning
- 1280x720, 3:1 binning
The VIDIOC_SUBDEV_S_FMT ioctl chooses among these 3 configurations the
one that matches the requested format.
Add cropping support via VIDIOC_SUBDEV_S_SELECTION: with target
V4L2_SEL_TGT_CROP to choose the captured area, with
V4L2_SEL_TGT_COMPOSE to choose the output resolution.
To maintain backward compatibility we also allow setting the compose
format via VIDIOC_SUBDEV_S_FMT. To obtain this, compose rect and
output format are computed in the common helper function
__imx274_change_compose(), which sets both to the same width/height
values.
Cropping also calls __imx274_change_compose() whenever cropping rect
size changes in order to reset the compose rect (and output format
size) for 1:1 binning.
Also change the names in enum imx274_mode from being resolution-based
to being binning-based (e.g. from IMX274_MODE_1920X1080 to
IMX274_MODE_BINNING_2_1). Without cropping, the two naming are
equivalent. With cropping, the resolution could be different,
e.g. using 2:1 binning mode to crop 1200x960 and output a 600x480
format. Using binning in the names avoids any misunderstanding. For
the same reason, replace the 'size' field in struct imx274_frmfmt with
'bin_ratio'.
Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Changed v3 -> v4:
- Set the binning via the SELECTION API (COMPOSE rect), but still
allow using VIDIOC_SUBDEV_S_FMT for backward compatibility.
- rename imx274_set_trimming -> imx274_apply_trimming for clarity
Changed v2 -> v3:
- Remove hard-coded HMAX reg setting from all modes, not only the
first one. HMAX is always computed and set in s_stream now.
- Reword commit log message.
Changed v1 -> v2:
- add "media: " prefix to commit message
---
drivers/media/i2c/imx274.c | 416 +++++++++++++++++++++++++++++++++++----------
1 file changed, 326 insertions(+), 90 deletions(-)
Comments
On Mon, Jun 11, 2018 at 01:35:39PM +0200, Luca Ceresoli wrote:
> Currently this driver does not support cropping. The supported modes
> are the following, all capturing the entire area:
>
> - 3840x2160, 1:1 binning (native sensor resolution)
> - 1920x1080, 2:1 binning
> - 1280x720, 3:1 binning
>
> The VIDIOC_SUBDEV_S_FMT ioctl chooses among these 3 configurations the
> one that matches the requested format.
>
> Add cropping support via VIDIOC_SUBDEV_S_SELECTION: with target
> V4L2_SEL_TGT_CROP to choose the captured area, with
> V4L2_SEL_TGT_COMPOSE to choose the output resolution.
>
> To maintain backward compatibility we also allow setting the compose
> format via VIDIOC_SUBDEV_S_FMT. To obtain this, compose rect and
> output format are computed in the common helper function
> __imx274_change_compose(), which sets both to the same width/height
> values.
>
> Cropping also calls __imx274_change_compose() whenever cropping rect
> size changes in order to reset the compose rect (and output format
> size) for 1:1 binning.
>
> Also change the names in enum imx274_mode from being resolution-based
> to being binning-based (e.g. from IMX274_MODE_1920X1080 to
> IMX274_MODE_BINNING_2_1). Without cropping, the two naming are
> equivalent. With cropping, the resolution could be different,
> e.g. using 2:1 binning mode to crop 1200x960 and output a 600x480
> format. Using binning in the names avoids any misunderstanding. For
> the same reason, replace the 'size' field in struct imx274_frmfmt with
> 'bin_ratio'.
>
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> ---
> Changed v3 -> v4:
> - Set the binning via the SELECTION API (COMPOSE rect), but still
> allow using VIDIOC_SUBDEV_S_FMT for backward compatibility.
> - rename imx274_set_trimming -> imx274_apply_trimming for clarity
>
> Changed v2 -> v3:
> - Remove hard-coded HMAX reg setting from all modes, not only the
> first one. HMAX is always computed and set in s_stream now.
> - Reword commit log message.
>
> Changed v1 -> v2:
> - add "media: " prefix to commit message
> ---
> drivers/media/i2c/imx274.c | 416 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 326 insertions(+), 90 deletions(-)
>
> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
> index 2c13961e9764..8bfc20a46b3d 100644
> --- a/drivers/media/i2c/imx274.c
> +++ b/drivers/media/i2c/imx274.c
> @@ -5,6 +5,7 @@
> *
> * Leon Luo <leonl@leopardimaging.com>
> * Edwin Zou <edwinz@leopardimaging.com>
> + * Luca Ceresoli <luca@lucaceresoli.net>
> *
> * This program is free software; you can redistribute it and/or modify it
> * under the terms and conditions of the GNU General Public License,
> @@ -19,6 +20,7 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <linux/kernel.h>
Alphabetical order, please.
> #include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/gpio.h>
> @@ -74,7 +76,7 @@
> */
> #define IMX274_MIN_EXPOSURE_TIME (4 * 260 / 72)
>
> -#define IMX274_DEFAULT_MODE IMX274_MODE_3840X2160
> +#define IMX274_DEFAULT_MODE IMX274_MODE_BINNING_OFF
> #define IMX274_MAX_WIDTH (3840)
> #define IMX274_MAX_HEIGHT (2160)
> #define IMX274_MAX_FRAME_RATE (120)
> @@ -111,6 +113,20 @@
> #define IMX274_SHR_REG_LSB 0x300C /* SHR */
> #define IMX274_SVR_REG_MSB 0x300F /* SVR */
> #define IMX274_SVR_REG_LSB 0x300E /* SVR */
> +#define IMX274_HTRIM_EN_REG 0x3037
> +#define IMX274_HTRIM_START_REG_LSB 0x3038
> +#define IMX274_HTRIM_START_REG_MSB 0x3039
> +#define IMX274_HTRIM_END_REG_LSB 0x303A
> +#define IMX274_HTRIM_END_REG_MSB 0x303B
> +#define IMX274_VWIDCUTEN_REG 0x30DD
> +#define IMX274_VWIDCUT_REG_LSB 0x30DE
> +#define IMX274_VWIDCUT_REG_MSB 0x30DF
> +#define IMX274_VWINPOS_REG_LSB 0x30E0
> +#define IMX274_VWINPOS_REG_MSB 0x30E1
> +#define IMX274_WRITE_VSIZE_REG_LSB 0x3130
> +#define IMX274_WRITE_VSIZE_REG_MSB 0x3131
> +#define IMX274_Y_OUT_SIZE_REG_LSB 0x3132
> +#define IMX274_Y_OUT_SIZE_REG_MSB 0x3133
> #define IMX274_VMAX_REG_1 0x30FA /* VMAX, MSB */
> #define IMX274_VMAX_REG_2 0x30F9 /* VMAX */
> #define IMX274_VMAX_REG_3 0x30F8 /* VMAX, LSB */
> @@ -141,9 +157,9 @@ static const struct regmap_config imx274_regmap_config = {
> };
>
> enum imx274_mode {
Is this still an appropriate name? imx274_binning?
> - IMX274_MODE_3840X2160,
> - IMX274_MODE_1920X1080,
> - IMX274_MODE_1280X720,
> + IMX274_MODE_BINNING_OFF,
> + IMX274_MODE_BINNING_2_1,
> + IMX274_MODE_BINNING_3_1,
> };
>
> /*
> @@ -152,8 +168,8 @@ enum imx274_mode {
> * These are the values to configure the sensor in one of the
> * implemented modes.
> *
> - * @size: recommended recording pixels
> * @init_regs: registers to initialize the mode
> + * @bin_ratio: downscale factor (e.g. 3 for 3:1 binning)
> * @min_frame_len: Minimum frame length for each mode (see "Frame Rate
> * Adjustment (CSI-2)" in the datasheet)
> * @min_SHR: Minimum SHR register value (see "Shutter Setting (CSI-2)" in the
> @@ -163,8 +179,8 @@ enum imx274_mode {
> * in Each Readout Drive Mode (CSI-2)" in the datasheet)
> */
> struct imx274_frmfmt {
> - struct v4l2_frmsize_discrete size;
> const struct reg_8 *init_regs;
> + unsigned int bin_ratio;
> int min_frame_len;
> int min_SHR;
> int max_fps;
> @@ -215,31 +231,14 @@ static const struct reg_8 imx274_mode1_3840x2160_raw10[] = {
> {0x3004, 0x01},
> {0x3005, 0x01},
> {0x3006, 0x00},
> - {0x3007, 0x02},
> + {0x3007, 0xa2},
>
> {0x3018, 0xA2}, /* output XVS, HVS */
>
> {0x306B, 0x05},
> {0x30E2, 0x01},
> - {0x30F6, 0x07}, /* HMAX, 263 */
> - {0x30F7, 0x01}, /* HMAX */
> -
> - {0x30dd, 0x01}, /* crop to 2160 */
> - {0x30de, 0x06},
> - {0x30df, 0x00},
> - {0x30e0, 0x12},
> - {0x30e1, 0x00},
> - {0x3037, 0x01}, /* to crop to 3840 */
> - {0x3038, 0x0c},
> - {0x3039, 0x00},
> - {0x303a, 0x0c},
> - {0x303b, 0x0f},
>
> {0x30EE, 0x01},
> - {0x3130, 0x86},
> - {0x3131, 0x08},
> - {0x3132, 0x7E},
> - {0x3133, 0x08},
> {0x3342, 0x0A},
> {0x3343, 0x00},
> {0x3344, 0x16},
> @@ -273,32 +272,14 @@ static const struct reg_8 imx274_mode3_1920x1080_raw10[] = {
> {0x3004, 0x02},
> {0x3005, 0x21},
> {0x3006, 0x00},
> - {0x3007, 0x11},
> + {0x3007, 0xb1},
>
> {0x3018, 0xA2}, /* output XVS, HVS */
>
> {0x306B, 0x05},
> {0x30E2, 0x02},
>
> - {0x30F6, 0x04}, /* HMAX, 260 */
> - {0x30F7, 0x01}, /* HMAX */
> -
> - {0x30dd, 0x01}, /* to crop to 1920x1080 */
> - {0x30de, 0x05},
> - {0x30df, 0x00},
> - {0x30e0, 0x04},
> - {0x30e1, 0x00},
> - {0x3037, 0x01},
> - {0x3038, 0x0c},
> - {0x3039, 0x00},
> - {0x303a, 0x0c},
> - {0x303b, 0x0f},
> -
> {0x30EE, 0x01},
> - {0x3130, 0x4E},
> - {0x3131, 0x04},
> - {0x3132, 0x46},
> - {0x3133, 0x04},
> {0x3342, 0x0A},
> {0x3343, 0x00},
> {0x3344, 0x1A},
> @@ -331,31 +312,14 @@ static const struct reg_8 imx274_mode5_1280x720_raw10[] = {
> {0x3004, 0x03},
> {0x3005, 0x31},
> {0x3006, 0x00},
> - {0x3007, 0x09},
> + {0x3007, 0xa9},
>
> {0x3018, 0xA2}, /* output XVS, HVS */
>
> {0x306B, 0x05},
> {0x30E2, 0x03},
>
> - {0x30F6, 0x04}, /* HMAX, 260 */
> - {0x30F7, 0x01}, /* HMAX */
> -
> - {0x30DD, 0x01},
> - {0x30DE, 0x07},
> - {0x30DF, 0x00},
> - {0x40E0, 0x04},
> - {0x30E1, 0x00},
> - {0x3030, 0xD4},
> - {0x3031, 0x02},
> - {0x3032, 0xD0},
> - {0x3033, 0x02},
> -
> {0x30EE, 0x01},
> - {0x3130, 0xE2},
> - {0x3131, 0x02},
> - {0x3132, 0xDE},
> - {0x3133, 0x02},
> {0x3342, 0x0A},
> {0x3343, 0x00},
> {0x3344, 0x1B},
> @@ -498,7 +462,7 @@ static const struct reg_8 imx274_tp_regs[] = {
> static const struct imx274_frmfmt imx274_formats[] = {
> {
> /* mode 1, 4K */
> - .size = {3840, 2160},
> + .bin_ratio = 1,
> .init_regs = imx274_mode1_3840x2160_raw10,
> .min_frame_len = 4550,
> .min_SHR = 12,
> @@ -507,7 +471,7 @@ static const struct imx274_frmfmt imx274_formats[] = {
> },
> {
> /* mode 3, 1080p */
> - .size = {1920, 1080},
> + .bin_ratio = 2,
> .init_regs = imx274_mode3_1920x1080_raw10,
> .min_frame_len = 2310,
> .min_SHR = 8,
> @@ -516,7 +480,7 @@ static const struct imx274_frmfmt imx274_formats[] = {
> },
> {
> /* mode 5, 720p */
> - .size = {1280, 720},
> + .bin_ratio = 3,
> .init_regs = imx274_mode5_1280x720_raw10,
> .min_frame_len = 2310,
> .min_SHR = 8,
> @@ -547,7 +511,10 @@ struct imx274_ctrls {
> * @pad: Media pad structure
> * @client: Pointer to I2C client
> * @ctrls: imx274 control structure
> + * @crop: rect to be captured
> + * @compose: compose rect, i.e. output resolution
> * @format: V4L2 media bus frame format structure
> + * (width and height are in sync with the compose rect)
> * @frame_rate: V4L2 frame rate structure
> * @regmap: Pointer to regmap structure
> * @reset_gpio: Pointer to reset gpio
> @@ -559,6 +526,8 @@ struct stimx274 {
> struct media_pad pad;
> struct i2c_client *client;
> struct imx274_ctrls ctrls;
> + struct v4l2_rect crop;
> + struct v4l2_rect compose;
> struct v4l2_mbus_framefmt format;
> struct v4l2_fract frame_interval;
> struct regmap *regmap;
> @@ -864,6 +833,80 @@ static int imx274_s_ctrl(struct v4l2_ctrl *ctrl)
> }
>
> /**
> + * Helper function to change binning and set both compose and format.
> + *
> + * We have two entry points to change binning: set_fmt and
> + * set_selection(COMPOSE). Both have to compute the new output size
> + * and set it in both the compose rect and the frame format size. We
> + * also need to do the same things after setting cropping to restore
> + * 1:1 binning.
> + *
> + * This function contains the common code for these three cases, it
> + * has many arguments in order to accomodate the needs of all of them.
> + *
> + * Must be called with imx274->lock locked.
> + *
> + * @imx274 The device object
Colon after the name of the argument. Same below.
> + * @cfg The pad config we are editing for TRY requests
> + * @which V4L2_SUBDEV_FORMAT_ACTIVE or V4L2_SUBDEV_FORMAT_TRY from the caller
> + * @width Input-output parameter: set to the desired width before
> + * the call, contains the chosen value after returning successfully
> + * @height Input-output parameter for height (see @width)
> + */
> +static int __imx274_change_compose(struct stimx274 *imx274,
> + struct v4l2_subdev_pad_config *cfg,
> + u32 which,
> + u32 *width,
> + u32 *height)
> +{
> + struct device *dev = &imx274->client->dev;
> + const struct v4l2_rect *cur_crop;
> + struct v4l2_mbus_framefmt *tgt_fmt;
> + struct v4l2_rect *tgt_compose;
> + unsigned int ratio; /* Binning ratio */
> +
> + dev_dbg(dev, "%s: request of %dx%d (%s)\n",
> + __func__, *width, *height,
> + (which == V4L2_SUBDEV_FORMAT_ACTIVE) ? "ACTIVE" : "TRY");
If these's a need for such debug prints, I think they'd be better added to
the framework than individual drivers.
> +
> + if (which == V4L2_SUBDEV_FORMAT_TRY) {
> + cur_crop = &cfg->try_crop;
> + tgt_compose = &cfg->try_compose;
> + tgt_fmt = &cfg->try_fmt;
> + } else {
> + cur_crop = &imx274->crop;
> + tgt_compose = &imx274->compose;
> + tgt_fmt = &imx274->format;
> + }
> +
> + /* Find ratio (maximize output resolution). Fallback to 1:1. */
> + for (ratio = 3; ratio > 1; ratio--)
> + if (*width <= DIV_ROUND_UP(cur_crop->width, ratio) &&
> + *height <= DIV_ROUND_UP(cur_crop->height, ratio))
> + break;
There are flags to direct the rounding behaviour. The default is the
nearest:
<URL:https://hverkuil.home.xs4all.nl/spec/uapi/v4l/v4l2-selection-flags.html>
The smiapp driver (as the only sensor driver) supports them.
> +
> + *width = cur_crop->width / ratio;
> + *height = cur_crop->height / ratio;
> +
> + if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
> + imx274->mode = &imx274_formats[ratio - 1];
> +
> + dev_dbg(dev, "%s: adjusted to %dx%d (%d:1 binning)\n",
> + __func__, *width, *height, ratio);
Could be here, for binning. %d -> %u --- it's unsigned.
> +
> + tgt_compose->top = 0;
> + tgt_compose->left = 0;
> + tgt_compose->width = *width;
> + tgt_compose->height = *height;
> +
> + tgt_fmt->width = *width;
> + tgt_fmt->height = *height;
> + tgt_fmt->field = V4L2_FIELD_NONE;
> +
> + return 0;
> +}
> +
> +/**
> * imx274_get_fmt - Get the pad format
> * @sd: Pointer to V4L2 Sub device structure
> * @cfg: Pointer to sub device pad information structure
> @@ -901,45 +944,228 @@ static int imx274_set_fmt(struct v4l2_subdev *sd,
> {
> struct v4l2_mbus_framefmt *fmt = &format->format;
> struct stimx274 *imx274 = to_imx274(sd);
> - struct i2c_client *client = imx274->client;
> - int index;
> -
> - dev_dbg(&client->dev,
> - "%s: width = %d height = %d code = %d\n",
> - __func__, fmt->width, fmt->height, fmt->code);
> + int err = 0;
>
> mutex_lock(&imx274->lock);
>
> - for (index = 0; index < ARRAY_SIZE(imx274_formats); index++) {
> - if (imx274_formats[index].size.width == fmt->width &&
> - imx274_formats[index].size.height == fmt->height)
> - break;
> - }
> -
> - if (index >= ARRAY_SIZE(imx274_formats)) {
> - /* default to first format */
> - index = 0;
> - }
> + err = __imx274_change_compose(imx274, cfg, format->which,
> + &fmt->width, &fmt->height);
>
> - imx274->mode = &imx274_formats[index];
> + if (err)
> + goto out;
>
> - if (fmt->width > IMX274_MAX_WIDTH)
> - fmt->width = IMX274_MAX_WIDTH;
> - if (fmt->height > IMX274_MAX_HEIGHT)
> - fmt->height = IMX274_MAX_HEIGHT;
> - fmt->width = fmt->width & (~IMX274_MASK_LSB_2_BITS);
> - fmt->height = fmt->height & (~IMX274_MASK_LSB_2_BITS);
> + /*
> + * __imx274_change_compose already set width and height in the
> + * applicable format, but we need to keep all other format
> + * values, so do a full copy here
> + */
> fmt->field = V4L2_FIELD_NONE;
> -
> if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> cfg->try_fmt = *fmt;
> else
> imx274->format = *fmt;
>
> +out:
> mutex_unlock(&imx274->lock);
> +
> + return err;
> +}
> +
> +static int imx274_get_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_selection *sel)
> +{
> + struct stimx274 *imx274 = to_imx274(sd);
> + const struct v4l2_rect *src_crop;
> + const struct v4l2_rect *src_compose;
> + int ret = 0;
> +
> + if (sel->pad != 0)
> + return -EINVAL;
> +
> + if (sel->target == V4L2_SEL_TGT_CROP_BOUNDS) {
> + sel->r.left = 0;
> + sel->r.top = 0;
> + sel->r.width = IMX274_MAX_WIDTH;
> + sel->r.height = IMX274_MAX_HEIGHT;
> + return 0;
> + }
> +
> + if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
> + src_crop = &cfg->try_crop;
> + src_compose = &cfg->try_compose;
> + } else {
> + src_crop = &imx274->crop;
> + src_compose = &imx274->compose;
> + }
> +
> + mutex_lock(&imx274->lock);
> +
> + switch (sel->target) {
> + case V4L2_SEL_TGT_CROP:
> + sel->r = *src_crop;
> + break;
> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> + sel->r.top = 0;
> + sel->r.left = 0;
> + sel->r.width = src_crop->width;
> + sel->r.height = src_crop->height;
> + break;
> + case V4L2_SEL_TGT_COMPOSE:
> + sel->r = *src_compose;
> + break;
> + default:
> + ret = -EINVAL;
> + }
> +
> + mutex_unlock(&imx274->lock);
> +
> + return ret;
> +}
> +
> +static int imx274_set_selection_crop(struct stimx274 *imx274,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_selection *sel)
> +{
> + struct device *dev = &imx274->client->dev;
> + struct v4l2_rect *tgt_crop;
> + struct v4l2_rect new_crop;
> + bool size_changed;
> +
> + /*
> + * h_step could be 12 or 24 depending on the binning. But we
> + * won't know the binning until we choose the mode later in
> + * imx274_set_fmt(). Thus let's be safe and use the most
> + * conservative value in all cases.
> + */
> + const int h_step = 24;
> +
> + dev_dbg(dev, "%s: request of (%d,%d)%dx%d (%s)\n", __func__,
> + sel->r.left, sel->r.top, sel->r.width, sel->r.height,
> + V4L2_SUBDEV_FORMAT_ACTIVE ? "ACTIVE" : "TRY");
> +
> + new_crop.width = rounddown(min_t(u32, sel->r.width, IMX274_MAX_WIDTH),
> + h_step);
> +
> + /* Constraint: HTRIMMING_END - HTRIMMING_START >= 144 */
> + if (new_crop.width < 144)
> + new_crop.width = 144;
> +
> + new_crop.left = min_t(u32, roundup(sel->r.left, h_step),
> + IMX274_MAX_WIDTH - new_crop.width);
> +
> + new_crop.height =
> + ALIGN(min_t(u32, sel->r.height, IMX274_MAX_HEIGHT), 2);
> +
> + new_crop.top = min_t(u32, ALIGN(sel->r.top, 2),
> + IMX274_MAX_HEIGHT - new_crop.height);
> +
> + dev_dbg(dev, "%s: adjusted to (%d,%d)%dx%d", __func__,
> + new_crop.left, new_crop.top, new_crop.width, new_crop.height);
> +
> + sel->r = new_crop;
> +
> + if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> + tgt_crop = &cfg->try_crop;
> + else
> + tgt_crop = &imx274->crop;
> +
> + mutex_lock(&imx274->lock);
> +
> + size_changed = (new_crop.width != tgt_crop->width ||
> + new_crop.height != tgt_crop->height);
> +
> + /* __imx274_change_compose needs the new size in *tgt_crop */
> + *tgt_crop = new_crop;
> +
> + /* if crop size changed then reset the output image size */
> + if (size_changed)
> + __imx274_change_compose(imx274, cfg, sel->which,
> + &new_crop.width, &new_crop.height);
> +
> + mutex_unlock(&imx274->lock);
> +
> return 0;
> }
>
> +static int imx274_set_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_selection *sel)
> +{
> + struct stimx274 *imx274 = to_imx274(sd);
> +
> + if (sel->pad != 0)
> + return -EINVAL;
> +
> + if (sel->target == V4L2_SEL_TGT_CROP)
> + return imx274_set_selection_crop(imx274, cfg, sel);
> +
> + if (sel->target == V4L2_SEL_TGT_COMPOSE) {
> + int err;
> +
> + mutex_lock(&imx274->lock);
> +
> + err = __imx274_change_compose(imx274, cfg, sel->which,
> + &sel->r.width, &sel->r.height);
> + /*
> + * __imx274_change_compose already set width and
> + * height in set->r, we still need to set top-left
> + */
> + if (!err) {
> + sel->r.top = 0;
> + sel->r.left = 0;
> + }
> +
> + mutex_unlock(&imx274->lock);
> +
> + return err;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int imx274_apply_trimming(struct stimx274 *imx274)
> +{
> + u32 h_start;
> + u32 h_end;
> + u32 hmax;
> + u32 v_cut;
> + s32 v_pos;
> + u32 write_v_size;
> + u32 y_out_size;
> + struct reg_8 regs[17];
> +
> + h_start = imx274->crop.left + 12;
> + h_end = h_start + imx274->crop.width;
> +
> + /* Use the minimum allowed value of HMAX */
> + /* Note: except in mode 1, (width / 16 + 23) is always < hmax_min */
> + /* Note: 260 is the minimum HMAX in all implemented modes */
> + hmax = max_t(u32, 260, (imx274->crop.width) / 16 + 23);
> +
> + /* invert v_pos if VFLIP */
> + v_pos = imx274->ctrls.vflip->cur.val ?
> + (-imx274->crop.top / 2) : (imx274->crop.top / 2);
> + v_cut = (IMX274_MAX_HEIGHT - imx274->crop.height) / 2;
> + write_v_size = imx274->crop.height + 22;
> + y_out_size = imx274->crop.height + 14;
> +
> + prepare_reg(®s[0], IMX274_HMAX_REG_LSB, hmax, 16);
> + prepare_reg(®s[2], IMX274_HTRIM_EN_REG, 1, 1);
> + prepare_reg(®s[3], IMX274_HTRIM_START_REG_LSB, h_start, 13);
> + prepare_reg(®s[5], IMX274_HTRIM_END_REG_LSB, h_end, 13);
> +
> + prepare_reg(®s[7], IMX274_VWIDCUTEN_REG, 1, 1);
> + prepare_reg(®s[8], IMX274_VWIDCUT_REG_LSB, v_cut, 11);
> + prepare_reg(®s[10], IMX274_VWINPOS_REG_LSB, v_pos, 12);
> + prepare_reg(®s[12], IMX274_WRITE_VSIZE_REG_LSB, write_v_size, 13);
> + prepare_reg(®s[14], IMX274_Y_OUT_SIZE_REG_LSB, y_out_size, 13);
> +
> + regs[16].addr = IMX274_TABLE_END;
> +
> + return imx274_write_table(imx274, regs);
> +}
> +
> /**
> * imx274_g_frame_interval - Get the frame interval
> * @sd: Pointer to V4L2 Sub device structure
> @@ -1080,6 +1306,10 @@ static int imx274_s_stream(struct v4l2_subdev *sd, int on)
> if (ret)
> goto fail;
>
> + ret = imx274_apply_trimming(imx274);
> + if (ret)
> + goto fail;
> +
> /*
> * update frame rate & expsoure. if the last mode is different,
> * HMAX could be changed. As the result, frame rate & exposure
> @@ -1589,6 +1819,8 @@ static int imx274_set_frame_interval(struct stimx274 *priv,
> static const struct v4l2_subdev_pad_ops imx274_pad_ops = {
> .get_fmt = imx274_get_fmt,
> .set_fmt = imx274_set_fmt,
> + .get_selection = imx274_get_selection,
> + .set_selection = imx274_set_selection,
> };
>
> static const struct v4l2_subdev_video_ops imx274_video_ops = {
> @@ -1634,8 +1866,12 @@ static int imx274_probe(struct i2c_client *client,
>
> /* initialize format */
> imx274->mode = &imx274_formats[IMX274_DEFAULT_MODE];
> - imx274->format.width = imx274->mode->size.width;
> - imx274->format.height = imx274->mode->size.height;
> + imx274->crop.width = IMX274_MAX_WIDTH;
> + imx274->crop.height = IMX274_MAX_HEIGHT;
> + imx274->compose.width = imx274->crop.width / imx274->mode->bin_ratio;
> + imx274->compose.height = imx274->crop.height / imx274->mode->bin_ratio;
> + imx274->format.width = imx274->compose.width;
> + imx274->format.height = imx274->compose.height;
> imx274->format.field = V4L2_FIELD_NONE;
> imx274->format.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> imx274->format.colorspace = V4L2_COLORSPACE_SRGB;
> --
> 2.7.4
>
Hi Sakari,
thanks for the review.
On 29/06/2018 10:04, Sakari Ailus wrote:
> On Mon, Jun 11, 2018 at 01:35:39PM +0200, Luca Ceresoli wrote:
>> Currently this driver does not support cropping. The supported modes
>> are the following, all capturing the entire area:
>>
>> - 3840x2160, 1:1 binning (native sensor resolution)
>> - 1920x1080, 2:1 binning
>> - 1280x720, 3:1 binning
>>
>> The VIDIOC_SUBDEV_S_FMT ioctl chooses among these 3 configurations the
>> one that matches the requested format.
>>
>> Add cropping support via VIDIOC_SUBDEV_S_SELECTION: with target
>> V4L2_SEL_TGT_CROP to choose the captured area, with
>> V4L2_SEL_TGT_COMPOSE to choose the output resolution.
>>
>> To maintain backward compatibility we also allow setting the compose
>> format via VIDIOC_SUBDEV_S_FMT. To obtain this, compose rect and
>> output format are computed in the common helper function
>> __imx274_change_compose(), which sets both to the same width/height
>> values.
>>
>> Cropping also calls __imx274_change_compose() whenever cropping rect
>> size changes in order to reset the compose rect (and output format
>> size) for 1:1 binning.
>>
>> Also change the names in enum imx274_mode from being resolution-based
>> to being binning-based (e.g. from IMX274_MODE_1920X1080 to
>> IMX274_MODE_BINNING_2_1). Without cropping, the two naming are
>> equivalent. With cropping, the resolution could be different,
>> e.g. using 2:1 binning mode to crop 1200x960 and output a 600x480
>> format. Using binning in the names avoids any misunderstanding. For
>> the same reason, replace the 'size' field in struct imx274_frmfmt with
>> 'bin_ratio'.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>>
>> ---
>> Changed v3 -> v4:
>> - Set the binning via the SELECTION API (COMPOSE rect), but still
>> allow using VIDIOC_SUBDEV_S_FMT for backward compatibility.
>> - rename imx274_set_trimming -> imx274_apply_trimming for clarity
>>
>> Changed v2 -> v3:
>> - Remove hard-coded HMAX reg setting from all modes, not only the
>> first one. HMAX is always computed and set in s_stream now.
>> - Reword commit log message.
>>
>> Changed v1 -> v2:
>> - add "media: " prefix to commit message
>> ---
>> drivers/media/i2c/imx274.c | 416 +++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 326 insertions(+), 90 deletions(-)
>>
>> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
>> index 2c13961e9764..8bfc20a46b3d 100644
>> --- a/drivers/media/i2c/imx274.c
>> +++ b/drivers/media/i2c/imx274.c
>> @@ -5,6 +5,7 @@
>> *
>> * Leon Luo <leonl@leopardimaging.com>
>> * Edwin Zou <edwinz@leopardimaging.com>
>> + * Luca Ceresoli <luca@lucaceresoli.net>
>> *
>> * This program is free software; you can redistribute it and/or modify it
>> * under the terms and conditions of the GNU General Public License,
>> @@ -19,6 +20,7 @@
>> * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> */
>>
>> +#include <linux/kernel.h>
>
> Alphabetical order, please.
Ok.
>> @@ -141,9 +157,9 @@ static const struct regmap_config imx274_regmap_config = {
>> };
>>
>> enum imx274_mode {
>
> Is this still an appropriate name? imx274_binning?
Yes, that would be coherent with the renaming of the individual fields.
>> - IMX274_MODE_3840X2160,
>> - IMX274_MODE_1920X1080,
>> - IMX274_MODE_1280X720,
>> + IMX274_MODE_BINNING_OFF,
>> + IMX274_MODE_BINNING_2_1,
>> + IMX274_MODE_BINNING_3_1,
>> };
>>
>> /*
[...]
>> @@ -559,6 +526,8 @@ struct stimx274 {
>> struct media_pad pad;
>> struct i2c_client *client;
>> struct imx274_ctrls ctrls;
>> + struct v4l2_rect crop;
>> + struct v4l2_rect compose;
>> struct v4l2_mbus_framefmt format;
I noticed the v4l2_rect compose can be removed. In this patch I keep
compose.width (and height) always equal to format.width (height). The
compose width (height) can be read from format.width (height). And since
compose.top and .left are always zero, compose carries no useful info.
Removing .compose will get less code to keep them in sync and no risk
for bugs that get them out of sync.
Note this is an implementation detail. No changes at the API level.
>> @@ -864,6 +833,80 @@ static int imx274_s_ctrl(struct v4l2_ctrl *ctrl)
>> }
>>
>> /**
>> + * Helper function to change binning and set both compose and format.
>> + *
>> + * We have two entry points to change binning: set_fmt and
>> + * set_selection(COMPOSE). Both have to compute the new output size
>> + * and set it in both the compose rect and the frame format size. We
>> + * also need to do the same things after setting cropping to restore
>> + * 1:1 binning.
>> + *
>> + * This function contains the common code for these three cases, it
>> + * has many arguments in order to accomodate the needs of all of them.
>> + *
>> + * Must be called with imx274->lock locked.
>> + *
>> + * @imx274 The device object
>
> Colon after the name of the argument. Same below.
Ouch. Thanks.
>> + * @cfg The pad config we are editing for TRY requests
>> + * @which V4L2_SUBDEV_FORMAT_ACTIVE or V4L2_SUBDEV_FORMAT_TRY from the caller
>> + * @width Input-output parameter: set to the desired width before
>> + * the call, contains the chosen value after returning successfully
>> + * @height Input-output parameter for height (see @width)
>> + */
>> +static int __imx274_change_compose(struct stimx274 *imx274,
>> + struct v4l2_subdev_pad_config *cfg,
>> + u32 which,
>> + u32 *width,
>> + u32 *height)
>> +{
>> + struct device *dev = &imx274->client->dev;
>> + const struct v4l2_rect *cur_crop;
>> + struct v4l2_mbus_framefmt *tgt_fmt;
>> + struct v4l2_rect *tgt_compose;
>> + unsigned int ratio; /* Binning ratio */
>> +
>> + dev_dbg(dev, "%s: request of %dx%d (%s)\n",
>> + __func__, *width, *height,
>> + (which == V4L2_SUBDEV_FORMAT_ACTIVE) ? "ACTIVE" : "TRY");
>
> If these's a need for such debug prints, I think they'd be better added to
> the framework than individual drivers.
Agreed, let's get rid of them.
>> +
>> + if (which == V4L2_SUBDEV_FORMAT_TRY) {
>> + cur_crop = &cfg->try_crop;
>> + tgt_compose = &cfg->try_compose;
>> + tgt_fmt = &cfg->try_fmt;
>> + } else {
>> + cur_crop = &imx274->crop;
>> + tgt_compose = &imx274->compose;
>> + tgt_fmt = &imx274->format;
>> + }
>> +
>> + /* Find ratio (maximize output resolution). Fallback to 1:1. */
>> + for (ratio = 3; ratio > 1; ratio--)
>> + if (*width <= DIV_ROUND_UP(cur_crop->width, ratio) &&
>> + *height <= DIV_ROUND_UP(cur_crop->height, ratio))
>> + break;
>
> There are flags to direct the rounding behaviour. The default is the
> nearest:
>
> <URL:https://hverkuil.home.xs4all.nl/spec/uapi/v4l/v4l2-selection-flags.html>
>
> The smiapp driver (as the only sensor driver) supports them.
Right, I will implement that.
>> +
>> + *width = cur_crop->width / ratio;
>> + *height = cur_crop->height / ratio;
>> +
>> + if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> + imx274->mode = &imx274_formats[ratio - 1];
>> +
>> + dev_dbg(dev, "%s: adjusted to %dx%d (%d:1 binning)\n",
>> + __func__, *width, *height, ratio);
>
> Could be here, for binning. %d -> %u --- it's unsigned.
Ok, in the nxt version this will look like:
dev_dbg(dev, "%s: selected %u:1 binning\n", __func__, ratio);
Bye,
@@ -5,6 +5,7 @@
*
* Leon Luo <leonl@leopardimaging.com>
* Edwin Zou <edwinz@leopardimaging.com>
+ * Luca Ceresoli <luca@lucaceresoli.net>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -19,6 +20,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <linux/kernel.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/gpio.h>
@@ -74,7 +76,7 @@
*/
#define IMX274_MIN_EXPOSURE_TIME (4 * 260 / 72)
-#define IMX274_DEFAULT_MODE IMX274_MODE_3840X2160
+#define IMX274_DEFAULT_MODE IMX274_MODE_BINNING_OFF
#define IMX274_MAX_WIDTH (3840)
#define IMX274_MAX_HEIGHT (2160)
#define IMX274_MAX_FRAME_RATE (120)
@@ -111,6 +113,20 @@
#define IMX274_SHR_REG_LSB 0x300C /* SHR */
#define IMX274_SVR_REG_MSB 0x300F /* SVR */
#define IMX274_SVR_REG_LSB 0x300E /* SVR */
+#define IMX274_HTRIM_EN_REG 0x3037
+#define IMX274_HTRIM_START_REG_LSB 0x3038
+#define IMX274_HTRIM_START_REG_MSB 0x3039
+#define IMX274_HTRIM_END_REG_LSB 0x303A
+#define IMX274_HTRIM_END_REG_MSB 0x303B
+#define IMX274_VWIDCUTEN_REG 0x30DD
+#define IMX274_VWIDCUT_REG_LSB 0x30DE
+#define IMX274_VWIDCUT_REG_MSB 0x30DF
+#define IMX274_VWINPOS_REG_LSB 0x30E0
+#define IMX274_VWINPOS_REG_MSB 0x30E1
+#define IMX274_WRITE_VSIZE_REG_LSB 0x3130
+#define IMX274_WRITE_VSIZE_REG_MSB 0x3131
+#define IMX274_Y_OUT_SIZE_REG_LSB 0x3132
+#define IMX274_Y_OUT_SIZE_REG_MSB 0x3133
#define IMX274_VMAX_REG_1 0x30FA /* VMAX, MSB */
#define IMX274_VMAX_REG_2 0x30F9 /* VMAX */
#define IMX274_VMAX_REG_3 0x30F8 /* VMAX, LSB */
@@ -141,9 +157,9 @@ static const struct regmap_config imx274_regmap_config = {
};
enum imx274_mode {
- IMX274_MODE_3840X2160,
- IMX274_MODE_1920X1080,
- IMX274_MODE_1280X720,
+ IMX274_MODE_BINNING_OFF,
+ IMX274_MODE_BINNING_2_1,
+ IMX274_MODE_BINNING_3_1,
};
/*
@@ -152,8 +168,8 @@ enum imx274_mode {
* These are the values to configure the sensor in one of the
* implemented modes.
*
- * @size: recommended recording pixels
* @init_regs: registers to initialize the mode
+ * @bin_ratio: downscale factor (e.g. 3 for 3:1 binning)
* @min_frame_len: Minimum frame length for each mode (see "Frame Rate
* Adjustment (CSI-2)" in the datasheet)
* @min_SHR: Minimum SHR register value (see "Shutter Setting (CSI-2)" in the
@@ -163,8 +179,8 @@ enum imx274_mode {
* in Each Readout Drive Mode (CSI-2)" in the datasheet)
*/
struct imx274_frmfmt {
- struct v4l2_frmsize_discrete size;
const struct reg_8 *init_regs;
+ unsigned int bin_ratio;
int min_frame_len;
int min_SHR;
int max_fps;
@@ -215,31 +231,14 @@ static const struct reg_8 imx274_mode1_3840x2160_raw10[] = {
{0x3004, 0x01},
{0x3005, 0x01},
{0x3006, 0x00},
- {0x3007, 0x02},
+ {0x3007, 0xa2},
{0x3018, 0xA2}, /* output XVS, HVS */
{0x306B, 0x05},
{0x30E2, 0x01},
- {0x30F6, 0x07}, /* HMAX, 263 */
- {0x30F7, 0x01}, /* HMAX */
-
- {0x30dd, 0x01}, /* crop to 2160 */
- {0x30de, 0x06},
- {0x30df, 0x00},
- {0x30e0, 0x12},
- {0x30e1, 0x00},
- {0x3037, 0x01}, /* to crop to 3840 */
- {0x3038, 0x0c},
- {0x3039, 0x00},
- {0x303a, 0x0c},
- {0x303b, 0x0f},
{0x30EE, 0x01},
- {0x3130, 0x86},
- {0x3131, 0x08},
- {0x3132, 0x7E},
- {0x3133, 0x08},
{0x3342, 0x0A},
{0x3343, 0x00},
{0x3344, 0x16},
@@ -273,32 +272,14 @@ static const struct reg_8 imx274_mode3_1920x1080_raw10[] = {
{0x3004, 0x02},
{0x3005, 0x21},
{0x3006, 0x00},
- {0x3007, 0x11},
+ {0x3007, 0xb1},
{0x3018, 0xA2}, /* output XVS, HVS */
{0x306B, 0x05},
{0x30E2, 0x02},
- {0x30F6, 0x04}, /* HMAX, 260 */
- {0x30F7, 0x01}, /* HMAX */
-
- {0x30dd, 0x01}, /* to crop to 1920x1080 */
- {0x30de, 0x05},
- {0x30df, 0x00},
- {0x30e0, 0x04},
- {0x30e1, 0x00},
- {0x3037, 0x01},
- {0x3038, 0x0c},
- {0x3039, 0x00},
- {0x303a, 0x0c},
- {0x303b, 0x0f},
-
{0x30EE, 0x01},
- {0x3130, 0x4E},
- {0x3131, 0x04},
- {0x3132, 0x46},
- {0x3133, 0x04},
{0x3342, 0x0A},
{0x3343, 0x00},
{0x3344, 0x1A},
@@ -331,31 +312,14 @@ static const struct reg_8 imx274_mode5_1280x720_raw10[] = {
{0x3004, 0x03},
{0x3005, 0x31},
{0x3006, 0x00},
- {0x3007, 0x09},
+ {0x3007, 0xa9},
{0x3018, 0xA2}, /* output XVS, HVS */
{0x306B, 0x05},
{0x30E2, 0x03},
- {0x30F6, 0x04}, /* HMAX, 260 */
- {0x30F7, 0x01}, /* HMAX */
-
- {0x30DD, 0x01},
- {0x30DE, 0x07},
- {0x30DF, 0x00},
- {0x40E0, 0x04},
- {0x30E1, 0x00},
- {0x3030, 0xD4},
- {0x3031, 0x02},
- {0x3032, 0xD0},
- {0x3033, 0x02},
-
{0x30EE, 0x01},
- {0x3130, 0xE2},
- {0x3131, 0x02},
- {0x3132, 0xDE},
- {0x3133, 0x02},
{0x3342, 0x0A},
{0x3343, 0x00},
{0x3344, 0x1B},
@@ -498,7 +462,7 @@ static const struct reg_8 imx274_tp_regs[] = {
static const struct imx274_frmfmt imx274_formats[] = {
{
/* mode 1, 4K */
- .size = {3840, 2160},
+ .bin_ratio = 1,
.init_regs = imx274_mode1_3840x2160_raw10,
.min_frame_len = 4550,
.min_SHR = 12,
@@ -507,7 +471,7 @@ static const struct imx274_frmfmt imx274_formats[] = {
},
{
/* mode 3, 1080p */
- .size = {1920, 1080},
+ .bin_ratio = 2,
.init_regs = imx274_mode3_1920x1080_raw10,
.min_frame_len = 2310,
.min_SHR = 8,
@@ -516,7 +480,7 @@ static const struct imx274_frmfmt imx274_formats[] = {
},
{
/* mode 5, 720p */
- .size = {1280, 720},
+ .bin_ratio = 3,
.init_regs = imx274_mode5_1280x720_raw10,
.min_frame_len = 2310,
.min_SHR = 8,
@@ -547,7 +511,10 @@ struct imx274_ctrls {
* @pad: Media pad structure
* @client: Pointer to I2C client
* @ctrls: imx274 control structure
+ * @crop: rect to be captured
+ * @compose: compose rect, i.e. output resolution
* @format: V4L2 media bus frame format structure
+ * (width and height are in sync with the compose rect)
* @frame_rate: V4L2 frame rate structure
* @regmap: Pointer to regmap structure
* @reset_gpio: Pointer to reset gpio
@@ -559,6 +526,8 @@ struct stimx274 {
struct media_pad pad;
struct i2c_client *client;
struct imx274_ctrls ctrls;
+ struct v4l2_rect crop;
+ struct v4l2_rect compose;
struct v4l2_mbus_framefmt format;
struct v4l2_fract frame_interval;
struct regmap *regmap;
@@ -864,6 +833,80 @@ static int imx274_s_ctrl(struct v4l2_ctrl *ctrl)
}
/**
+ * Helper function to change binning and set both compose and format.
+ *
+ * We have two entry points to change binning: set_fmt and
+ * set_selection(COMPOSE). Both have to compute the new output size
+ * and set it in both the compose rect and the frame format size. We
+ * also need to do the same things after setting cropping to restore
+ * 1:1 binning.
+ *
+ * This function contains the common code for these three cases, it
+ * has many arguments in order to accomodate the needs of all of them.
+ *
+ * Must be called with imx274->lock locked.
+ *
+ * @imx274 The device object
+ * @cfg The pad config we are editing for TRY requests
+ * @which V4L2_SUBDEV_FORMAT_ACTIVE or V4L2_SUBDEV_FORMAT_TRY from the caller
+ * @width Input-output parameter: set to the desired width before
+ * the call, contains the chosen value after returning successfully
+ * @height Input-output parameter for height (see @width)
+ */
+static int __imx274_change_compose(struct stimx274 *imx274,
+ struct v4l2_subdev_pad_config *cfg,
+ u32 which,
+ u32 *width,
+ u32 *height)
+{
+ struct device *dev = &imx274->client->dev;
+ const struct v4l2_rect *cur_crop;
+ struct v4l2_mbus_framefmt *tgt_fmt;
+ struct v4l2_rect *tgt_compose;
+ unsigned int ratio; /* Binning ratio */
+
+ dev_dbg(dev, "%s: request of %dx%d (%s)\n",
+ __func__, *width, *height,
+ (which == V4L2_SUBDEV_FORMAT_ACTIVE) ? "ACTIVE" : "TRY");
+
+ if (which == V4L2_SUBDEV_FORMAT_TRY) {
+ cur_crop = &cfg->try_crop;
+ tgt_compose = &cfg->try_compose;
+ tgt_fmt = &cfg->try_fmt;
+ } else {
+ cur_crop = &imx274->crop;
+ tgt_compose = &imx274->compose;
+ tgt_fmt = &imx274->format;
+ }
+
+ /* Find ratio (maximize output resolution). Fallback to 1:1. */
+ for (ratio = 3; ratio > 1; ratio--)
+ if (*width <= DIV_ROUND_UP(cur_crop->width, ratio) &&
+ *height <= DIV_ROUND_UP(cur_crop->height, ratio))
+ break;
+
+ *width = cur_crop->width / ratio;
+ *height = cur_crop->height / ratio;
+
+ if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
+ imx274->mode = &imx274_formats[ratio - 1];
+
+ dev_dbg(dev, "%s: adjusted to %dx%d (%d:1 binning)\n",
+ __func__, *width, *height, ratio);
+
+ tgt_compose->top = 0;
+ tgt_compose->left = 0;
+ tgt_compose->width = *width;
+ tgt_compose->height = *height;
+
+ tgt_fmt->width = *width;
+ tgt_fmt->height = *height;
+ tgt_fmt->field = V4L2_FIELD_NONE;
+
+ return 0;
+}
+
+/**
* imx274_get_fmt - Get the pad format
* @sd: Pointer to V4L2 Sub device structure
* @cfg: Pointer to sub device pad information structure
@@ -901,45 +944,228 @@ static int imx274_set_fmt(struct v4l2_subdev *sd,
{
struct v4l2_mbus_framefmt *fmt = &format->format;
struct stimx274 *imx274 = to_imx274(sd);
- struct i2c_client *client = imx274->client;
- int index;
-
- dev_dbg(&client->dev,
- "%s: width = %d height = %d code = %d\n",
- __func__, fmt->width, fmt->height, fmt->code);
+ int err = 0;
mutex_lock(&imx274->lock);
- for (index = 0; index < ARRAY_SIZE(imx274_formats); index++) {
- if (imx274_formats[index].size.width == fmt->width &&
- imx274_formats[index].size.height == fmt->height)
- break;
- }
-
- if (index >= ARRAY_SIZE(imx274_formats)) {
- /* default to first format */
- index = 0;
- }
+ err = __imx274_change_compose(imx274, cfg, format->which,
+ &fmt->width, &fmt->height);
- imx274->mode = &imx274_formats[index];
+ if (err)
+ goto out;
- if (fmt->width > IMX274_MAX_WIDTH)
- fmt->width = IMX274_MAX_WIDTH;
- if (fmt->height > IMX274_MAX_HEIGHT)
- fmt->height = IMX274_MAX_HEIGHT;
- fmt->width = fmt->width & (~IMX274_MASK_LSB_2_BITS);
- fmt->height = fmt->height & (~IMX274_MASK_LSB_2_BITS);
+ /*
+ * __imx274_change_compose already set width and height in the
+ * applicable format, but we need to keep all other format
+ * values, so do a full copy here
+ */
fmt->field = V4L2_FIELD_NONE;
-
if (format->which == V4L2_SUBDEV_FORMAT_TRY)
cfg->try_fmt = *fmt;
else
imx274->format = *fmt;
+out:
mutex_unlock(&imx274->lock);
+
+ return err;
+}
+
+static int imx274_get_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_selection *sel)
+{
+ struct stimx274 *imx274 = to_imx274(sd);
+ const struct v4l2_rect *src_crop;
+ const struct v4l2_rect *src_compose;
+ int ret = 0;
+
+ if (sel->pad != 0)
+ return -EINVAL;
+
+ if (sel->target == V4L2_SEL_TGT_CROP_BOUNDS) {
+ sel->r.left = 0;
+ sel->r.top = 0;
+ sel->r.width = IMX274_MAX_WIDTH;
+ sel->r.height = IMX274_MAX_HEIGHT;
+ return 0;
+ }
+
+ if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
+ src_crop = &cfg->try_crop;
+ src_compose = &cfg->try_compose;
+ } else {
+ src_crop = &imx274->crop;
+ src_compose = &imx274->compose;
+ }
+
+ mutex_lock(&imx274->lock);
+
+ switch (sel->target) {
+ case V4L2_SEL_TGT_CROP:
+ sel->r = *src_crop;
+ break;
+ case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+ sel->r.top = 0;
+ sel->r.left = 0;
+ sel->r.width = src_crop->width;
+ sel->r.height = src_crop->height;
+ break;
+ case V4L2_SEL_TGT_COMPOSE:
+ sel->r = *src_compose;
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ mutex_unlock(&imx274->lock);
+
+ return ret;
+}
+
+static int imx274_set_selection_crop(struct stimx274 *imx274,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_selection *sel)
+{
+ struct device *dev = &imx274->client->dev;
+ struct v4l2_rect *tgt_crop;
+ struct v4l2_rect new_crop;
+ bool size_changed;
+
+ /*
+ * h_step could be 12 or 24 depending on the binning. But we
+ * won't know the binning until we choose the mode later in
+ * imx274_set_fmt(). Thus let's be safe and use the most
+ * conservative value in all cases.
+ */
+ const int h_step = 24;
+
+ dev_dbg(dev, "%s: request of (%d,%d)%dx%d (%s)\n", __func__,
+ sel->r.left, sel->r.top, sel->r.width, sel->r.height,
+ V4L2_SUBDEV_FORMAT_ACTIVE ? "ACTIVE" : "TRY");
+
+ new_crop.width = rounddown(min_t(u32, sel->r.width, IMX274_MAX_WIDTH),
+ h_step);
+
+ /* Constraint: HTRIMMING_END - HTRIMMING_START >= 144 */
+ if (new_crop.width < 144)
+ new_crop.width = 144;
+
+ new_crop.left = min_t(u32, roundup(sel->r.left, h_step),
+ IMX274_MAX_WIDTH - new_crop.width);
+
+ new_crop.height =
+ ALIGN(min_t(u32, sel->r.height, IMX274_MAX_HEIGHT), 2);
+
+ new_crop.top = min_t(u32, ALIGN(sel->r.top, 2),
+ IMX274_MAX_HEIGHT - new_crop.height);
+
+ dev_dbg(dev, "%s: adjusted to (%d,%d)%dx%d", __func__,
+ new_crop.left, new_crop.top, new_crop.width, new_crop.height);
+
+ sel->r = new_crop;
+
+ if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
+ tgt_crop = &cfg->try_crop;
+ else
+ tgt_crop = &imx274->crop;
+
+ mutex_lock(&imx274->lock);
+
+ size_changed = (new_crop.width != tgt_crop->width ||
+ new_crop.height != tgt_crop->height);
+
+ /* __imx274_change_compose needs the new size in *tgt_crop */
+ *tgt_crop = new_crop;
+
+ /* if crop size changed then reset the output image size */
+ if (size_changed)
+ __imx274_change_compose(imx274, cfg, sel->which,
+ &new_crop.width, &new_crop.height);
+
+ mutex_unlock(&imx274->lock);
+
return 0;
}
+static int imx274_set_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_selection *sel)
+{
+ struct stimx274 *imx274 = to_imx274(sd);
+
+ if (sel->pad != 0)
+ return -EINVAL;
+
+ if (sel->target == V4L2_SEL_TGT_CROP)
+ return imx274_set_selection_crop(imx274, cfg, sel);
+
+ if (sel->target == V4L2_SEL_TGT_COMPOSE) {
+ int err;
+
+ mutex_lock(&imx274->lock);
+
+ err = __imx274_change_compose(imx274, cfg, sel->which,
+ &sel->r.width, &sel->r.height);
+ /*
+ * __imx274_change_compose already set width and
+ * height in set->r, we still need to set top-left
+ */
+ if (!err) {
+ sel->r.top = 0;
+ sel->r.left = 0;
+ }
+
+ mutex_unlock(&imx274->lock);
+
+ return err;
+ }
+
+ return -EINVAL;
+}
+
+static int imx274_apply_trimming(struct stimx274 *imx274)
+{
+ u32 h_start;
+ u32 h_end;
+ u32 hmax;
+ u32 v_cut;
+ s32 v_pos;
+ u32 write_v_size;
+ u32 y_out_size;
+ struct reg_8 regs[17];
+
+ h_start = imx274->crop.left + 12;
+ h_end = h_start + imx274->crop.width;
+
+ /* Use the minimum allowed value of HMAX */
+ /* Note: except in mode 1, (width / 16 + 23) is always < hmax_min */
+ /* Note: 260 is the minimum HMAX in all implemented modes */
+ hmax = max_t(u32, 260, (imx274->crop.width) / 16 + 23);
+
+ /* invert v_pos if VFLIP */
+ v_pos = imx274->ctrls.vflip->cur.val ?
+ (-imx274->crop.top / 2) : (imx274->crop.top / 2);
+ v_cut = (IMX274_MAX_HEIGHT - imx274->crop.height) / 2;
+ write_v_size = imx274->crop.height + 22;
+ y_out_size = imx274->crop.height + 14;
+
+ prepare_reg(®s[0], IMX274_HMAX_REG_LSB, hmax, 16);
+ prepare_reg(®s[2], IMX274_HTRIM_EN_REG, 1, 1);
+ prepare_reg(®s[3], IMX274_HTRIM_START_REG_LSB, h_start, 13);
+ prepare_reg(®s[5], IMX274_HTRIM_END_REG_LSB, h_end, 13);
+
+ prepare_reg(®s[7], IMX274_VWIDCUTEN_REG, 1, 1);
+ prepare_reg(®s[8], IMX274_VWIDCUT_REG_LSB, v_cut, 11);
+ prepare_reg(®s[10], IMX274_VWINPOS_REG_LSB, v_pos, 12);
+ prepare_reg(®s[12], IMX274_WRITE_VSIZE_REG_LSB, write_v_size, 13);
+ prepare_reg(®s[14], IMX274_Y_OUT_SIZE_REG_LSB, y_out_size, 13);
+
+ regs[16].addr = IMX274_TABLE_END;
+
+ return imx274_write_table(imx274, regs);
+}
+
/**
* imx274_g_frame_interval - Get the frame interval
* @sd: Pointer to V4L2 Sub device structure
@@ -1080,6 +1306,10 @@ static int imx274_s_stream(struct v4l2_subdev *sd, int on)
if (ret)
goto fail;
+ ret = imx274_apply_trimming(imx274);
+ if (ret)
+ goto fail;
+
/*
* update frame rate & expsoure. if the last mode is different,
* HMAX could be changed. As the result, frame rate & exposure
@@ -1589,6 +1819,8 @@ static int imx274_set_frame_interval(struct stimx274 *priv,
static const struct v4l2_subdev_pad_ops imx274_pad_ops = {
.get_fmt = imx274_get_fmt,
.set_fmt = imx274_set_fmt,
+ .get_selection = imx274_get_selection,
+ .set_selection = imx274_set_selection,
};
static const struct v4l2_subdev_video_ops imx274_video_ops = {
@@ -1634,8 +1866,12 @@ static int imx274_probe(struct i2c_client *client,
/* initialize format */
imx274->mode = &imx274_formats[IMX274_DEFAULT_MODE];
- imx274->format.width = imx274->mode->size.width;
- imx274->format.height = imx274->mode->size.height;
+ imx274->crop.width = IMX274_MAX_WIDTH;
+ imx274->crop.height = IMX274_MAX_HEIGHT;
+ imx274->compose.width = imx274->crop.width / imx274->mode->bin_ratio;
+ imx274->compose.height = imx274->crop.height / imx274->mode->bin_ratio;
+ imx274->format.width = imx274->compose.width;
+ imx274->format.height = imx274->compose.height;
imx274->format.field = V4L2_FIELD_NONE;
imx274->format.code = MEDIA_BUS_FMT_SRGGB10_1X10;
imx274->format.colorspace = V4L2_COLORSPACE_SRGB;