From 092b6ffffb76cf930be6a7f3cb960b3b786676dc Mon Sep 17 00:00:00 2001 From: Dave Stevenson Date: Tue, 7 Jul 2020 10:31:53 +0100 Subject: [PATCH 1/5] media: i2c: imx290: Explicitly set v&h blank on mode change __v4l2_ctrl_modify_range only updates the current value should it be invalid within the new range. That can leave modes producing odd frame rates. Explicitly update the HBLANK and VBLANK values so that on mode change we revert to the default frame rate for the mode. Signed-off-by: Dave Stevenson --- drivers/media/i2c/imx290.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index 9431c2b917ed5a..0aa3940c577ea4 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -796,17 +796,23 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, __v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate, imx290_calc_pixel_rate(imx290)); - if (imx290->hblank) + if (imx290->hblank) { __v4l2_ctrl_modify_range(imx290->hblank, imx290->hmax_min - mode->width, IMX290_HMAX_MAX - mode->width, 1, mode->hmax - mode->width); - if (imx290->vblank) + __v4l2_ctrl_s_ctrl(imx290->hblank, + mode->hmax - mode->width); + } + if (imx290->vblank) { __v4l2_ctrl_modify_range(imx290->vblank, mode->vmax - mode->height, IMX290_VMAX_MAX - mode->height, 1, mode->vmax - mode->height); + __v4l2_ctrl_s_ctrl(imx290->vblank, + mode->vmax - mode->height); + } if (imx290->exposure) __v4l2_ctrl_modify_range(imx290->exposure, mode->vmax - mode->height, From a8847accf260571f62de38c949eacf7241bb0e86 Mon Sep 17 00:00:00 2001 From: Dave Stevenson Date: Tue, 7 Jul 2020 11:23:48 +0100 Subject: [PATCH 2/5] media: i2c: imx290: Add support for g_selection to report cropping Userspace needs to know the cropping arrangements for each mode, so expose this through g_selection. Signed-off-by: Dave Stevenson --- drivers/media/i2c/imx290.c | 84 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index 0aa3940c577ea4..5b7083b7050e91 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -61,6 +61,13 @@ enum imx290_clk_index { #define IMX290_PGCTRL_THRU BIT(1) #define IMX290_PGCTRL_MODE(n) ((n) << 4) +#define IMX290_NATIVE_WIDTH 1945U +#define IMX290_NATIVE_HEIGHT 1109U +#define IMX290_PIXEL_ARRAY_LEFT 4U +#define IMX290_PIXEL_ARRAY_TOP 12U +#define IMX290_PIXEL_ARRAY_WIDTH 1937U +#define IMX290_PIXEL_ARRAY_HEIGHT 1097U + static const char * const imx290_supply_name[] = { "vdda", "vddd", @@ -80,6 +87,7 @@ struct imx290_mode { u32 hmax; u32 vmax; u8 link_freq_index; + struct v4l2_rect crop; const struct imx290_regval *data; u32 data_size; @@ -384,6 +392,12 @@ static const struct imx290_mode imx290_modes_2lanes[] = { .hmax = 0x1130, .vmax = 0x0465, .link_freq_index = FREQ_INDEX_1080P, + .crop = { + .left = 4 + 8, + .top = 12 + 8, + .width = 1920, + .height = 1080, + }, .data = imx290_1080p_settings, .data_size = ARRAY_SIZE(imx290_1080p_settings), .clk_data = { @@ -398,6 +412,12 @@ static const struct imx290_mode imx290_modes_2lanes[] = { .hmax = 0x19c8, .vmax = 0x02ee, .link_freq_index = FREQ_INDEX_720P, + .crop = { + .left = 4 + 8 + 320, + .top = 12 + 8 + 180, + .width = 1280, + .height = 720, + }, .data = imx290_720p_settings, .data_size = ARRAY_SIZE(imx290_720p_settings), .clk_data = { @@ -415,6 +435,12 @@ static const struct imx290_mode imx290_modes_4lanes[] = { .hmax = 0x0898, .vmax = 0x0465, .link_freq_index = FREQ_INDEX_1080P, + .crop = { + .left = 4 + 8, + .top = 12 + 8, + .width = 1920, + .height = 1080, + }, .data = imx290_1080p_settings, .data_size = ARRAY_SIZE(imx290_1080p_settings), .clk_data = { @@ -429,6 +455,12 @@ static const struct imx290_mode imx290_modes_4lanes[] = { .hmax = 0x0ce4, .vmax = 0x02ee, .link_freq_index = FREQ_INDEX_720P, + .crop = { + .left = 4 + 8 + 320, + .top = 12 + 8 + 180, + .width = 1280, + .height = 720, + }, .data = imx290_720p_settings, .data_size = ARRAY_SIZE(imx290_720p_settings), .clk_data = { @@ -875,6 +907,57 @@ static int imx290_write_current_format(struct imx290 *imx290) return 0; } +static const struct v4l2_rect * +__imx290_get_pad_crop(struct imx290 *imx290, struct v4l2_subdev_pad_config *cfg, + unsigned int pad, enum v4l2_subdev_format_whence which) +{ + switch (which) { + case V4L2_SUBDEV_FORMAT_TRY: + return v4l2_subdev_get_try_crop(&imx290->sd, cfg, pad); + case V4L2_SUBDEV_FORMAT_ACTIVE: + return &imx290->current_mode->crop; + } + + return NULL; +} + +static int imx290_get_selection(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_selection *sel) +{ + switch (sel->target) { + case V4L2_SEL_TGT_CROP: { + struct imx290 *imx290 = to_imx290(sd); + + mutex_lock(&imx290->lock); + sel->r = *__imx290_get_pad_crop(imx290, cfg, sel->pad, + sel->which); + mutex_unlock(&imx290->lock); + + return 0; + } + + case V4L2_SEL_TGT_NATIVE_SIZE: + sel->r.top = 0; + sel->r.left = 0; + sel->r.width = IMX290_NATIVE_WIDTH; + sel->r.height = IMX290_NATIVE_HEIGHT; + + return 0; + + case V4L2_SEL_TGT_CROP_DEFAULT: + case V4L2_SEL_TGT_CROP_BOUNDS: + sel->r.top = IMX290_PIXEL_ARRAY_TOP; + sel->r.left = IMX290_PIXEL_ARRAY_LEFT; + sel->r.width = IMX290_PIXEL_ARRAY_WIDTH; + sel->r.height = IMX290_PIXEL_ARRAY_HEIGHT; + + return 0; + } + + return -EINVAL; +} + /* Start streaming */ static int imx290_start_streaming(struct imx290 *imx290) { @@ -1073,6 +1156,7 @@ static const struct v4l2_subdev_pad_ops imx290_pad_ops = { .enum_frame_size = imx290_enum_frame_size, .get_fmt = imx290_get_fmt, .set_fmt = imx290_set_fmt, + .get_selection = imx290_get_selection, }; static const struct v4l2_subdev_ops imx290_subdev_ops = { From 3f305c83f242c3cae098a8c202edaba20d1673ab Mon Sep 17 00:00:00 2001 From: Dave Stevenson Date: Tue, 7 Jul 2020 11:51:26 +0100 Subject: [PATCH 3/5] media: i2c: imx290: Set the colorspace fields in the format The colorspace fields were left untouched in imx290_set_fmt which lead to a v4l2-compliance failure. Signed-off-by: Dave Stevenson --- drivers/media/i2c/imx290.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index 5b7083b7050e91..0057e0b9891234 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -813,6 +813,14 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, fmt->format.code = imx290->formats[i].code; fmt->format.field = V4L2_FIELD_NONE; + fmt->format.colorspace = V4L2_COLORSPACE_SRGB; + fmt->format.ycbcr_enc = + V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->format.colorspace); + fmt->format.quantization = + V4L2_MAP_QUANTIZATION_DEFAULT(true, fmt->format.colorspace, + fmt->format.ycbcr_enc); + fmt->format.xfer_func = + V4L2_MAP_XFER_FUNC_DEFAULT(fmt->format.colorspace); if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) { format = v4l2_subdev_get_try_format(sd, cfg, fmt->pad); From b292cd8585ca71d0b13646d0ef127c1cec636b43 Mon Sep 17 00:00:00 2001 From: Dave Stevenson Date: Tue, 7 Jul 2020 14:23:40 +0100 Subject: [PATCH 4/5] media: bcm2835-unicam: Reinstate V4L2_CAP_READWRITE in the caps v4l2-compliance throws a failure if the device doesn't advertise V4L2_CAP_READWRITE but allows read or write operations. We do support read, so reinstate the flag. Signed-off-by: Dave Stevenson --- drivers/media/platform/bcm2835/bcm2835-unicam.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/bcm2835/bcm2835-unicam.c b/drivers/media/platform/bcm2835/bcm2835-unicam.c index 3b082587dd14df..6b7780ec8ec6ce 100644 --- a/drivers/media/platform/bcm2835/bcm2835-unicam.c +++ b/drivers/media/platform/bcm2835/bcm2835-unicam.c @@ -2416,8 +2416,8 @@ static int register_node(struct unicam_device *unicam, struct unicam_node *node, vdev->queue = q; vdev->lock = &node->lock; vdev->device_caps = (pad_id == IMAGE_PAD) ? - (V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING) : - (V4L2_CAP_META_CAPTURE | V4L2_CAP_STREAMING); + V4L2_CAP_VIDEO_CAPTURE : V4L2_CAP_META_CAPTURE; + vdev->device_caps |= V4L2_CAP_READWRITE | V4L2_CAP_STREAMING; /* Define the device names */ snprintf(vdev->name, sizeof(vdev->name), "%s-%s", UNICAM_MODULE_NAME, From 824ccc5dad08b151a83fdaed292530399ba97a2d Mon Sep 17 00:00:00 2001 From: Dave Stevenson Date: Tue, 7 Jul 2020 14:52:43 +0100 Subject: [PATCH 5/5] media: bcm2835-unicam: Ensure type is VIDEO_CAPTURE in [g|s]_selection [g|s]_selection pass in a buffer type that needs to be validated before passing on to the sensor subdev. Signed-off-by: Dave Stevenson --- drivers/media/platform/bcm2835/bcm2835-unicam.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/platform/bcm2835/bcm2835-unicam.c b/drivers/media/platform/bcm2835/bcm2835-unicam.c index 6b7780ec8ec6ce..c2b9d89f0ae521 100644 --- a/drivers/media/platform/bcm2835/bcm2835-unicam.c +++ b/drivers/media/platform/bcm2835/bcm2835-unicam.c @@ -1880,6 +1880,9 @@ static int unicam_s_selection(struct file *file, void *priv, .r = sel->r, }; + if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + return -EINVAL; + return v4l2_subdev_call(dev->sensor, pad, set_selection, NULL, &sdsel); } @@ -1894,6 +1897,9 @@ static int unicam_g_selection(struct file *file, void *priv, }; int ret; + if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) + return -EINVAL; + ret = v4l2_subdev_call(dev->sensor, pad, get_selection, NULL, &sdsel); if (!ret) sel->r = sdsel.r;