From patchwork Sat Jan 14 17:17:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 642825 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5D0E9C46467 for ; Sat, 14 Jan 2023 17:18:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230193AbjANRSH (ORCPT ); Sat, 14 Jan 2023 12:18:07 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40376 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229989AbjANRSF (ORCPT ); Sat, 14 Jan 2023 12:18:05 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C7F7DA5F9 for ; Sat, 14 Jan 2023 09:18:03 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 474C64D4; Sat, 14 Jan 2023 18:18:02 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1673716682; bh=jwgiiDsxdjj5KyqfgNdGkH9gDakEfr7pfNUZ8CZSjfs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=W/SYGywIaE0uyyukC4ULfsNF13U9KuDe3hydi7+LJHN85GIVXgAA2Zdy6d4PK7/x/ RnLRd05MdDaC2DeGzs+5jnfOCTQDyvUR2arKg4LWZN09K+ao9e6CZmmqXane269Bni m7Dqi3wuRkqMdgtENGdv2m1FWYk5rmesnGzTeCBg= From: Laurent Pinchart To: linux-media@vger.kernel.org Cc: Sakari Ailus , Manivannan Sadhasivam , Alexander Stein , Dave Stevenson Subject: [PATCH v2 01/17] media: i2c: imx290: Group functions in sections Date: Sat, 14 Jan 2023 19:17:46 +0200 Message-Id: <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.38.2 In-Reply-To: <20230114171727.13830-1-laurent.pinchart@ideasonboard.com> References: <20230114171727.13830-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Move functions around to group them in logical sections, in order to improve readability. As a result, the IMX290_NUM_SUPPLIES macro has to be changed. No other code change is included, only moves. Signed-off-by: Laurent Pinchart Reviewed-by: Alexander Stein Reviewed-by: Alexander Stein Reviewed-by: Alexander Stein --- drivers/media/i2c/imx290.c | 626 +++++++++++++++++++------------------ 1 file changed, 325 insertions(+), 301 deletions(-) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index 218ded13fd80..ca2fa57c28fe 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -152,13 +152,7 @@ #define IMX290_PIXEL_ARRAY_RECORDING_WIDTH 1920 #define IMX290_PIXEL_ARRAY_RECORDING_HEIGHT 1080 -static const char * const imx290_supply_name[] = { - "vdda", - "vddd", - "vdddo", -}; - -#define IMX290_NUM_SUPPLIES ARRAY_SIZE(imx290_supply_name) +#define IMX290_NUM_SUPPLIES 3 struct imx290_regval { u32 reg; @@ -199,31 +193,14 @@ struct imx290 { struct mutex lock; }; -struct imx290_pixfmt { - u32 code; - u8 bpp; -}; +static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd) +{ + return container_of(_sd, struct imx290, sd); +} -static const struct imx290_pixfmt imx290_formats[] = { - { MEDIA_BUS_FMT_SRGGB10_1X10, 10 }, - { MEDIA_BUS_FMT_SRGGB12_1X12, 12 }, -}; - -static const struct regmap_config imx290_regmap_config = { - .reg_bits = 16, - .val_bits = 8, -}; - -static const char * const imx290_test_pattern_menu[] = { - "Disabled", - "Sequence Pattern 1", - "Horizontal Color-bar Chart", - "Vertical Color-bar Chart", - "Sequence Pattern 2", - "Gradation Pattern 1", - "Gradation Pattern 2", - "000/555h Toggle Pattern", -}; +/* ----------------------------------------------------------------------------- + * Modes and formats + */ static const struct imx290_regval imx290_global_init_settings[] = { { IMX290_CTRL_07, IMX290_WINMODE_1080P }, @@ -438,10 +415,19 @@ static inline int imx290_modes_num(const struct imx290 *imx290) return ARRAY_SIZE(imx290_modes_4lanes); } -static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd) -{ - return container_of(_sd, struct imx290, sd); -} +struct imx290_pixfmt { + u32 code; + u8 bpp; +}; + +static const struct imx290_pixfmt imx290_formats[] = { + { MEDIA_BUS_FMT_SRGGB10_1X10, 10 }, + { MEDIA_BUS_FMT_SRGGB12_1X12, 12 }, +}; + +/* ----------------------------------------------------------------------------- + * Register access + */ static int __always_unused imx290_read(struct imx290 *imx290, u32 addr, u32 *value) { @@ -501,18 +487,94 @@ static int imx290_set_register_array(struct imx290 *imx290, return 0; } -/* Stop streaming */ -static int imx290_stop_streaming(struct imx290 *imx290) +static int imx290_set_data_lanes(struct imx290 *imx290) { - int ret = 0; + int ret = 0, laneval, frsel; - imx290_write(imx290, IMX290_STANDBY, 0x01, &ret); + switch (imx290->nlanes) { + case 2: + laneval = 0x01; + frsel = 0x02; + break; + case 4: + laneval = 0x03; + frsel = 0x01; + break; + default: + /* + * We should never hit this since the data lane count is + * validated in probe itself + */ + dev_err(imx290->dev, "Lane configuration not supported\n"); + return -EINVAL; + } - msleep(30); + imx290_write(imx290, IMX290_PHY_LANE_NUM, laneval, &ret); + imx290_write(imx290, IMX290_CSI_LANE_MODE, laneval, &ret); + imx290_write(imx290, IMX290_FR_FDG_SEL, frsel, &ret); - return imx290_write(imx290, IMX290_XMSTA, 0x01, &ret); + return ret; } +static int imx290_write_current_format(struct imx290 *imx290) +{ + int ret; + + switch (imx290->current_format.code) { + case MEDIA_BUS_FMT_SRGGB10_1X10: + ret = imx290_set_register_array(imx290, imx290_10bit_settings, + ARRAY_SIZE( + imx290_10bit_settings)); + if (ret < 0) { + dev_err(imx290->dev, "Could not set format registers\n"); + return ret; + } + break; + case MEDIA_BUS_FMT_SRGGB12_1X12: + ret = imx290_set_register_array(imx290, imx290_12bit_settings, + ARRAY_SIZE( + imx290_12bit_settings)); + if (ret < 0) { + dev_err(imx290->dev, "Could not set format registers\n"); + return ret; + } + break; + default: + dev_err(imx290->dev, "Unknown pixel format\n"); + return -EINVAL; + } + + return 0; +} + +static inline u8 imx290_get_link_freq_index(struct imx290 *imx290) +{ + return imx290->current_mode->link_freq_index; +} + +static s64 imx290_get_link_freq(struct imx290 *imx290) +{ + u8 index = imx290_get_link_freq_index(imx290); + + return *(imx290_link_freqs_ptr(imx290) + index); +} + +static u64 imx290_calc_pixel_rate(struct imx290 *imx290) +{ + s64 link_freq = imx290_get_link_freq(imx290); + u8 nlanes = imx290->nlanes; + u64 pixel_rate; + + /* pixel rate = link_freq * 2 * nr_of_lanes / bits_per_sample */ + pixel_rate = link_freq * 2 * nlanes; + do_div(pixel_rate, imx290->bpp); + return pixel_rate; +} + +/* ---------------------------------------------------------------------------- + * Controls + */ + static int imx290_set_ctrl(struct v4l2_ctrl *ctrl) { struct imx290 *imx290 = container_of(ctrl->handler, @@ -566,6 +628,187 @@ static const struct v4l2_ctrl_ops imx290_ctrl_ops = { .s_ctrl = imx290_set_ctrl, }; +static const char * const imx290_test_pattern_menu[] = { + "Disabled", + "Sequence Pattern 1", + "Horizontal Color-bar Chart", + "Vertical Color-bar Chart", + "Sequence Pattern 2", + "Gradation Pattern 1", + "Gradation Pattern 2", + "000/555h Toggle Pattern", +}; + +static int imx290_ctrl_init(struct imx290 *imx290) +{ + struct v4l2_fwnode_device_properties props; + unsigned int blank; + int ret; + + ret = v4l2_fwnode_device_parse(imx290->dev, &props); + if (ret < 0) + return ret; + + v4l2_ctrl_handler_init(&imx290->ctrls, 9); + imx290->ctrls.lock = &imx290->lock; + + /* + * The sensor has an analog gain and a digital gain, both controlled + * through a single gain value, expressed in 0.3dB increments. Values + * from 0.0dB (0) to 30.0dB (100) apply analog gain only, higher values + * up to 72.0dB (240) add further digital gain. Limit the range to + * analog gain only, support for digital gain can be added separately + * if needed. + * + * The IMX327 and IMX462 are largely compatible with the IMX290, but + * have an analog gain range of 0.0dB to 29.4dB and 42dB of digital + * gain. When support for those sensors gets added to the driver, the + * gain control should be adjusted accordingly. + */ + v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, + V4L2_CID_ANALOGUE_GAIN, 0, 100, 1, 0); + + v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, + V4L2_CID_EXPOSURE, 1, IMX290_VMAX_DEFAULT - 2, 1, + IMX290_VMAX_DEFAULT - 2); + + imx290->link_freq = + v4l2_ctrl_new_int_menu(&imx290->ctrls, &imx290_ctrl_ops, + V4L2_CID_LINK_FREQ, + imx290_link_freqs_num(imx290) - 1, 0, + imx290_link_freqs_ptr(imx290)); + if (imx290->link_freq) + imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; + + imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, + V4L2_CID_PIXEL_RATE, + 1, INT_MAX, 1, + imx290_calc_pixel_rate(imx290)); + + v4l2_ctrl_new_std_menu_items(&imx290->ctrls, &imx290_ctrl_ops, + V4L2_CID_TEST_PATTERN, + ARRAY_SIZE(imx290_test_pattern_menu) - 1, + 0, 0, imx290_test_pattern_menu); + + blank = imx290->current_mode->hmax - imx290->current_mode->width; + imx290->hblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, + V4L2_CID_HBLANK, blank, blank, 1, + blank); + if (imx290->hblank) + imx290->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; + + blank = IMX290_VMAX_DEFAULT - imx290->current_mode->height; + imx290->vblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, + V4L2_CID_VBLANK, blank, blank, 1, + blank); + if (imx290->vblank) + imx290->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; + + v4l2_ctrl_new_fwnode_properties(&imx290->ctrls, &imx290_ctrl_ops, + &props); + + imx290->sd.ctrl_handler = &imx290->ctrls; + + if (imx290->ctrls.error) { + ret = imx290->ctrls.error; + v4l2_ctrl_handler_free(&imx290->ctrls); + return ret; + } + + return 0; +} + +/* ---------------------------------------------------------------------------- + * Subdev operations + */ + +/* Start streaming */ +static int imx290_start_streaming(struct imx290 *imx290) +{ + int ret; + + /* Set init register settings */ + ret = imx290_set_register_array(imx290, imx290_global_init_settings, + ARRAY_SIZE( + imx290_global_init_settings)); + if (ret < 0) { + dev_err(imx290->dev, "Could not set init registers\n"); + return ret; + } + + /* Apply the register values related to current frame format */ + ret = imx290_write_current_format(imx290); + if (ret < 0) { + dev_err(imx290->dev, "Could not set frame format\n"); + return ret; + } + + /* Apply default values of current mode */ + ret = imx290_set_register_array(imx290, imx290->current_mode->data, + imx290->current_mode->data_size); + if (ret < 0) { + dev_err(imx290->dev, "Could not set current mode\n"); + return ret; + } + + ret = imx290_write(imx290, IMX290_HMAX, imx290->current_mode->hmax, + NULL); + if (ret) + return ret; + + /* Apply customized values from user */ + ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler); + if (ret) { + dev_err(imx290->dev, "Could not sync v4l2 controls\n"); + return ret; + } + + imx290_write(imx290, IMX290_STANDBY, 0x00, &ret); + + msleep(30); + + /* Start streaming */ + return imx290_write(imx290, IMX290_XMSTA, 0x00, &ret); +} + +/* Stop streaming */ +static int imx290_stop_streaming(struct imx290 *imx290) +{ + int ret = 0; + + imx290_write(imx290, IMX290_STANDBY, 0x01, &ret); + + msleep(30); + + return imx290_write(imx290, IMX290_XMSTA, 0x01, &ret); +} + +static int imx290_set_stream(struct v4l2_subdev *sd, int enable) +{ + struct imx290 *imx290 = to_imx290(sd); + int ret = 0; + + if (enable) { + ret = pm_runtime_resume_and_get(imx290->dev); + if (ret < 0) + goto unlock_and_return; + + ret = imx290_start_streaming(imx290); + if (ret) { + dev_err(imx290->dev, "Start stream failed\n"); + pm_runtime_put(imx290->dev); + goto unlock_and_return; + } + } else { + imx290_stop_streaming(imx290); + pm_runtime_put(imx290->dev); + } + +unlock_and_return: + + return ret; +} + static struct v4l2_mbus_framefmt * imx290_get_pad_format(struct imx290 *imx290, struct v4l2_subdev_state *state, u32 which) @@ -627,30 +870,6 @@ static int imx290_get_fmt(struct v4l2_subdev *sd, return 0; } -static inline u8 imx290_get_link_freq_index(struct imx290 *imx290) -{ - return imx290->current_mode->link_freq_index; -} - -static s64 imx290_get_link_freq(struct imx290 *imx290) -{ - u8 index = imx290_get_link_freq_index(imx290); - - return *(imx290_link_freqs_ptr(imx290) + index); -} - -static u64 imx290_calc_pixel_rate(struct imx290 *imx290) -{ - s64 link_freq = imx290_get_link_freq(imx290); - u8 nlanes = imx290->nlanes; - u64 pixel_rate; - - /* pixel rate = link_freq * 2 * nr_of_lanes / bits_per_sample */ - pixel_rate = link_freq * 2 * nlanes; - do_div(pixel_rate, imx290->bpp); - return pixel_rate; -} - static int imx290_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, struct v4l2_subdev_format *fmt) @@ -774,151 +993,31 @@ static int imx290_entity_init_cfg(struct v4l2_subdev *subdev, return 0; } -static int imx290_write_current_format(struct imx290 *imx290) -{ - int ret; - - switch (imx290->current_format.code) { - case MEDIA_BUS_FMT_SRGGB10_1X10: - ret = imx290_set_register_array(imx290, imx290_10bit_settings, - ARRAY_SIZE( - imx290_10bit_settings)); - if (ret < 0) { - dev_err(imx290->dev, "Could not set format registers\n"); - return ret; - } - break; - case MEDIA_BUS_FMT_SRGGB12_1X12: - ret = imx290_set_register_array(imx290, imx290_12bit_settings, - ARRAY_SIZE( - imx290_12bit_settings)); - if (ret < 0) { - dev_err(imx290->dev, "Could not set format registers\n"); - return ret; - } - break; - default: - dev_err(imx290->dev, "Unknown pixel format\n"); - return -EINVAL; - } - - return 0; -} - -/* Start streaming */ -static int imx290_start_streaming(struct imx290 *imx290) -{ - int ret; - - /* Set init register settings */ - ret = imx290_set_register_array(imx290, imx290_global_init_settings, - ARRAY_SIZE( - imx290_global_init_settings)); - if (ret < 0) { - dev_err(imx290->dev, "Could not set init registers\n"); - return ret; - } - - /* Apply the register values related to current frame format */ - ret = imx290_write_current_format(imx290); - if (ret < 0) { - dev_err(imx290->dev, "Could not set frame format\n"); - return ret; - } - - /* Apply default values of current mode */ - ret = imx290_set_register_array(imx290, imx290->current_mode->data, - imx290->current_mode->data_size); - if (ret < 0) { - dev_err(imx290->dev, "Could not set current mode\n"); - return ret; - } - - ret = imx290_write(imx290, IMX290_HMAX, imx290->current_mode->hmax, - NULL); - if (ret) - return ret; - - /* Apply customized values from user */ - ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler); - if (ret) { - dev_err(imx290->dev, "Could not sync v4l2 controls\n"); - return ret; - } - - imx290_write(imx290, IMX290_STANDBY, 0x00, &ret); - - msleep(30); - - /* Start streaming */ - return imx290_write(imx290, IMX290_XMSTA, 0x00, &ret); -} - -static int imx290_set_stream(struct v4l2_subdev *sd, int enable) -{ - struct imx290 *imx290 = to_imx290(sd); - int ret = 0; - - if (enable) { - ret = pm_runtime_resume_and_get(imx290->dev); - if (ret < 0) - goto unlock_and_return; - - ret = imx290_start_streaming(imx290); - if (ret) { - dev_err(imx290->dev, "Start stream failed\n"); - pm_runtime_put(imx290->dev); - goto unlock_and_return; - } - } else { - imx290_stop_streaming(imx290); - pm_runtime_put(imx290->dev); - } - -unlock_and_return: - - return ret; -} - -static int imx290_get_regulators(struct device *dev, struct imx290 *imx290) -{ - unsigned int i; - - for (i = 0; i < ARRAY_SIZE(imx290->supplies); i++) - imx290->supplies[i].supply = imx290_supply_name[i]; - - return devm_regulator_bulk_get(dev, ARRAY_SIZE(imx290->supplies), - imx290->supplies); -} - -static int imx290_set_data_lanes(struct imx290 *imx290) -{ - int ret = 0, laneval, frsel; - - switch (imx290->nlanes) { - case 2: - laneval = 0x01; - frsel = 0x02; - break; - case 4: - laneval = 0x03; - frsel = 0x01; - break; - default: - /* - * We should never hit this since the data lane count is - * validated in probe itself - */ - dev_err(imx290->dev, "Lane configuration not supported\n"); - return -EINVAL; - } - - imx290_write(imx290, IMX290_PHY_LANE_NUM, laneval, &ret); - imx290_write(imx290, IMX290_CSI_LANE_MODE, laneval, &ret); - imx290_write(imx290, IMX290_FR_FDG_SEL, frsel, &ret); - - return ret; -} +static const struct v4l2_subdev_video_ops imx290_video_ops = { + .s_stream = imx290_set_stream, +}; + +static const struct v4l2_subdev_pad_ops imx290_pad_ops = { + .init_cfg = imx290_entity_init_cfg, + .enum_mbus_code = imx290_enum_mbus_code, + .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 = { + .video = &imx290_video_ops, + .pad = &imx290_pad_ops, +}; + +static const struct media_entity_operations imx290_subdev_entity_ops = { + .link_validate = v4l2_subdev_link_validate, +}; + +/* ---------------------------------------------------------------------------- + * Power management + */ static int imx290_power_on(struct device *dev) { @@ -966,105 +1065,30 @@ static const struct dev_pm_ops imx290_pm_ops = { SET_RUNTIME_PM_OPS(imx290_power_off, imx290_power_on, NULL) }; -static const struct v4l2_subdev_video_ops imx290_video_ops = { - .s_stream = imx290_set_stream, -}; - -static const struct v4l2_subdev_pad_ops imx290_pad_ops = { - .init_cfg = imx290_entity_init_cfg, - .enum_mbus_code = imx290_enum_mbus_code, - .enum_frame_size = imx290_enum_frame_size, - .get_fmt = imx290_get_fmt, - .set_fmt = imx290_set_fmt, - .get_selection = imx290_get_selection, -}; +/* ---------------------------------------------------------------------------- + * Probe & remove + */ -static const struct v4l2_subdev_ops imx290_subdev_ops = { - .video = &imx290_video_ops, - .pad = &imx290_pad_ops, +static const struct regmap_config imx290_regmap_config = { + .reg_bits = 16, + .val_bits = 8, }; -static const struct media_entity_operations imx290_subdev_entity_ops = { - .link_validate = v4l2_subdev_link_validate, +static const char * const imx290_supply_name[IMX290_NUM_SUPPLIES] = { + "vdda", + "vddd", + "vdddo", }; -static int imx290_ctrl_init(struct imx290 *imx290) +static int imx290_get_regulators(struct device *dev, struct imx290 *imx290) { - struct v4l2_fwnode_device_properties props; - unsigned int blank; - int ret; + unsigned int i; - ret = v4l2_fwnode_device_parse(imx290->dev, &props); - if (ret < 0) - return ret; + for (i = 0; i < ARRAY_SIZE(imx290->supplies); i++) + imx290->supplies[i].supply = imx290_supply_name[i]; - v4l2_ctrl_handler_init(&imx290->ctrls, 9); - imx290->ctrls.lock = &imx290->lock; - - /* - * The sensor has an analog gain and a digital gain, both controlled - * through a single gain value, expressed in 0.3dB increments. Values - * from 0.0dB (0) to 30.0dB (100) apply analog gain only, higher values - * up to 72.0dB (240) add further digital gain. Limit the range to - * analog gain only, support for digital gain can be added separately - * if needed. - * - * The IMX327 and IMX462 are largely compatible with the IMX290, but - * have an analog gain range of 0.0dB to 29.4dB and 42dB of digital - * gain. When support for those sensors gets added to the driver, the - * gain control should be adjusted accordingly. - */ - v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, - V4L2_CID_ANALOGUE_GAIN, 0, 100, 1, 0); - - v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, - V4L2_CID_EXPOSURE, 1, IMX290_VMAX_DEFAULT - 2, 1, - IMX290_VMAX_DEFAULT - 2); - - imx290->link_freq = - v4l2_ctrl_new_int_menu(&imx290->ctrls, &imx290_ctrl_ops, - V4L2_CID_LINK_FREQ, - imx290_link_freqs_num(imx290) - 1, 0, - imx290_link_freqs_ptr(imx290)); - if (imx290->link_freq) - imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; - - imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, - V4L2_CID_PIXEL_RATE, - 1, INT_MAX, 1, - imx290_calc_pixel_rate(imx290)); - - v4l2_ctrl_new_std_menu_items(&imx290->ctrls, &imx290_ctrl_ops, - V4L2_CID_TEST_PATTERN, - ARRAY_SIZE(imx290_test_pattern_menu) - 1, - 0, 0, imx290_test_pattern_menu); - - blank = imx290->current_mode->hmax - imx290->current_mode->width; - imx290->hblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, - V4L2_CID_HBLANK, blank, blank, 1, - blank); - if (imx290->hblank) - imx290->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; - - blank = IMX290_VMAX_DEFAULT - imx290->current_mode->height; - imx290->vblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, - V4L2_CID_VBLANK, blank, blank, 1, - blank); - if (imx290->vblank) - imx290->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; - - v4l2_ctrl_new_fwnode_properties(&imx290->ctrls, &imx290_ctrl_ops, - &props); - - imx290->sd.ctrl_handler = &imx290->ctrls; - - if (imx290->ctrls.error) { - ret = imx290->ctrls.error; - v4l2_ctrl_handler_free(&imx290->ctrls); - return ret; - } - - return 0; + return devm_regulator_bulk_get(dev, ARRAY_SIZE(imx290->supplies), + imx290->supplies); } /* From patchwork Sat Jan 14 17:17:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 642824 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8FA05C6379F for ; Sat, 14 Jan 2023 17:18:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229676AbjANRSK (ORCPT ); Sat, 14 Jan 2023 12:18:10 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40406 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230320AbjANRSI (ORCPT ); Sat, 14 Jan 2023 12:18:08 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5392CB752 for ; Sat, 14 Jan 2023 09:18:07 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 646016DB; Sat, 14 Jan 2023 18:18:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1673716685; bh=S0mr0mEajdJgdF2jgPcvyRlGG8s78Gxjj2kL3TqgIwU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=P34kj2CEIi2JoL6NZgm8a1NJ3iiq4GhxHGdxqjbWaL4yWH75fjY7XUOO70H96tcqc LKOjVxJq6zg7aCqzCkbZjt/LhEtdMPnDZOIHF355lppdgLNhpCkeefOnPjou7be90M nthlLurRgGiRrrBsWolD/p4wbbn6vQdlb+DqORgE= From: Laurent Pinchart To: linux-media@vger.kernel.org Cc: Sakari Ailus , Manivannan Sadhasivam , Alexander Stein , Dave Stevenson Subject: [PATCH v2 03/17] media: i2c: imx290: Factor out control update code to a function Date: Sat, 14 Jan 2023 19:17:48 +0200 Message-Id: <20230114171802.13878-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.38.2 In-Reply-To: <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> References: <20230114171727.13830-1-laurent.pinchart@ideasonboard.com> <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Move the control update code to a separate function to group it with all the control-related code and make imx290_set_fmt() more readable. Signed-off-by: Laurent Pinchart --- Changes since v1: - Correctly handle the case where imx290_ctrl_update() gets called before controls are initialized --- drivers/media/i2c/imx290.c | 43 ++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index 5529bd39238f..991e7285c40c 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -639,6 +639,28 @@ static const char * const imx290_test_pattern_menu[] = { "000/555h Toggle Pattern", }; +static void imx290_ctrl_update(struct imx290 *imx290, + const struct imx290_mode *mode) +{ + unsigned int hblank = mode->hmax - mode->width; + unsigned int vblank = IMX290_VMAX_DEFAULT - mode->height; + + /* + * This function may be called from imx290_set_fmt() before controls + * get created by imx290_ctrl_init(). Return immediately in that case. + */ + if (!imx290->ctrls.lock) + return; + + __v4l2_ctrl_s_ctrl(imx290->link_freq, + imx290_get_link_freq_index(imx290)); + __v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate, + imx290_calc_pixel_rate(imx290)); + + __v4l2_ctrl_modify_range(imx290->hblank, hblank, hblank, 1, hblank); + __v4l2_ctrl_modify_range(imx290->vblank, vblank, vblank, 1, vblank); +} + static int imx290_ctrl_init(struct imx290 *imx290) { struct v4l2_fwnode_device_properties props; @@ -904,26 +926,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, imx290->current_mode = mode; imx290->bpp = imx290_formats[i].bpp; - if (imx290->link_freq) - __v4l2_ctrl_s_ctrl(imx290->link_freq, - imx290_get_link_freq_index(imx290)); - if (imx290->pixel_rate) - __v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate, - imx290_calc_pixel_rate(imx290)); - - if (imx290->hblank) { - unsigned int hblank = mode->hmax - mode->width; - - __v4l2_ctrl_modify_range(imx290->hblank, hblank, hblank, - 1, hblank); - } - - if (imx290->vblank) { - unsigned int vblank = IMX290_VMAX_DEFAULT - mode->height; - - __v4l2_ctrl_modify_range(imx290->vblank, vblank, vblank, - 1, vblank); - } + imx290_ctrl_update(imx290, mode); } *format = fmt->format; From patchwork Sat Jan 14 17:17:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 642823 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B82A9C46467 for ; Sat, 14 Jan 2023 17:18:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230340AbjANRSN (ORCPT ); Sat, 14 Jan 2023 12:18:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40422 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230317AbjANRSK (ORCPT ); Sat, 14 Jan 2023 12:18:10 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A080AB752 for ; Sat, 14 Jan 2023 09:18:09 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2E070E68; Sat, 14 Jan 2023 18:18:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1673716688; bh=fZcgwZ2+qwVm9Gp2ryh7Yc5kNZDjqv2s154Hvv51GPU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QS3MtMm06Xdzm+rj/HUY3JGxlfZZyMXR1Yr8zfbOqdiRmSUu94iauW5T3lnpcJMXy sWycTB0h7LZfYe/Jr84OsPPn9iK+qN4aeWHTz8lnUd559pFn6DQgeolysgdkuM0zRH gtgb5G1BO+X3LT/bxb7o1hnKUTEuWFDN6TID+BSM= From: Laurent Pinchart To: linux-media@vger.kernel.org Cc: Sakari Ailus , Manivannan Sadhasivam , Alexander Stein , Dave Stevenson Subject: [PATCH v2 05/17] media: i2c: imx290: Pass format and mode to imx290_calc_pixel_rate() Date: Sat, 14 Jan 2023 19:17:50 +0200 Message-Id: <20230114171802.13878-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.38.2 In-Reply-To: <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> References: <20230114171727.13830-1-laurent.pinchart@ideasonboard.com> <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Avoid accessing the imx290 current_format and current_mode fields in imx290_calc_pixel_rate() to prepare for the removal of those fields. Among the two callers of the function, imx290_ctrl_update() has an explicit mode pointer already, and we can also give it a format pointer. Use those explicitly. While at it, inline the imx290_get_link_freq() function in imx290_calc_pixel_rate() as it is only called there. Signed-off-by: Laurent Pinchart Reviewed-by: Alexander Stein --- Changes since v1: - Drop format argument from imx290_calc_pixel_rate() --- drivers/media/i2c/imx290.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index 4ad6eab4f2e2..8f141df74e2f 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -547,21 +547,15 @@ static int imx290_write_current_format(struct imx290 *imx290) return 0; } -static s64 imx290_get_link_freq(struct imx290 *imx290) +static u64 imx290_calc_pixel_rate(struct imx290 *imx290, + const struct v4l2_mbus_framefmt *format, + const struct imx290_mode *mode) { - u8 index = imx290->current_mode->link_freq_index; - - return *(imx290_link_freqs_ptr(imx290) + index); -} - -static u64 imx290_calc_pixel_rate(struct imx290 *imx290) -{ - s64 link_freq = imx290_get_link_freq(imx290); - u8 nlanes = imx290->nlanes; + s64 link_freq = imx290_link_freqs_ptr(imx290)[mode->link_freq_index]; u64 pixel_rate; /* pixel rate = link_freq * 2 * nr_of_lanes / bits_per_sample */ - pixel_rate = link_freq * 2 * nlanes; + pixel_rate = link_freq * 2 * imx290->nlanes; do_div(pixel_rate, imx290->bpp); return pixel_rate; } @@ -635,6 +629,7 @@ static const char * const imx290_test_pattern_menu[] = { }; static void imx290_ctrl_update(struct imx290 *imx290, + const struct v4l2_mbus_framefmt *format, const struct imx290_mode *mode) { unsigned int hblank = mode->hmax - mode->width; @@ -649,7 +644,7 @@ static void imx290_ctrl_update(struct imx290 *imx290, __v4l2_ctrl_s_ctrl(imx290->link_freq, mode->link_freq_index); __v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate, - imx290_calc_pixel_rate(imx290)); + imx290_calc_pixel_rate(imx290, format, mode)); __v4l2_ctrl_modify_range(imx290->hblank, hblank, hblank, 1, hblank); __v4l2_ctrl_modify_range(imx290->vblank, vblank, vblank, 1, vblank); @@ -659,6 +654,7 @@ static int imx290_ctrl_init(struct imx290 *imx290) { struct v4l2_fwnode_device_properties props; unsigned int blank; + u64 pixel_rate; int ret; ret = v4l2_fwnode_device_parse(imx290->dev, &props); @@ -696,10 +692,11 @@ static int imx290_ctrl_init(struct imx290 *imx290) if (imx290->link_freq) imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY; + pixel_rate = imx290_calc_pixel_rate(imx290, &imx290->current_format, + imx290->current_mode); imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops, V4L2_CID_PIXEL_RATE, - 1, INT_MAX, 1, - imx290_calc_pixel_rate(imx290)); + 1, INT_MAX, 1, pixel_rate); v4l2_ctrl_new_std_menu_items(&imx290->ctrls, &imx290_ctrl_ops, V4L2_CID_TEST_PATTERN, @@ -920,7 +917,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, imx290->current_mode = mode; imx290->bpp = imx290_formats[i].bpp; - imx290_ctrl_update(imx290, mode); + imx290_ctrl_update(imx290, &fmt->format, mode); } *format = fmt->format; From patchwork Sat Jan 14 17:17:52 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 642822 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C1080C3DA78 for ; Sat, 14 Jan 2023 17:18:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230344AbjANRSQ (ORCPT ); Sat, 14 Jan 2023 12:18:16 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40462 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230336AbjANRSN (ORCPT ); Sat, 14 Jan 2023 12:18:13 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 89A92B762 for ; Sat, 14 Jan 2023 09:18:12 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 174B44DD; Sat, 14 Jan 2023 18:18:11 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1673716691; bh=GkHNl/vEqXrWZl6GyNKNShGMhT09G4TRj0d0QsJ0+Q0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DQHs7Ui/Kll9VK9e4S/90ZWXrCmSmdaBs08dK4Ef4R2dcI3poir8LzyPzHuozFVq/ Xd2v6VKC3ZRwZe1JGd00X05cry/k47q6Zspj0NQRn2aNQ3l4YxtDNYRD6yuxbdbtVW EkjZfW6MrIq5miVvQO6Tr67G2jvYpT5ItxM4vQvU= From: Laurent Pinchart To: linux-media@vger.kernel.org Cc: Sakari Ailus , Manivannan Sadhasivam , Alexander Stein , Dave Stevenson Subject: [PATCH v2 07/17] media: i2c: imx290: Factor out black level setting to a function Date: Sat, 14 Jan 2023 19:17:52 +0200 Message-Id: <20230114171802.13878-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.38.2 In-Reply-To: <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> References: <20230114171727.13830-1-laurent.pinchart@ideasonboard.com> <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org The black level programmed in the BLKLEVEL register depends on the output format. The black level value computation is currently performed in imx290_set_ctrl(), in addition to having different black level values in the output-specific register value tables. Move it to a separate function to simplify the imx290_set_ctrl() code. Signed-off-by: Laurent Pinchart Reviewed-by: Alexander Stein Reviewed-by: Dave Stevenson --- Changes since v1: - Drop bpp variable in imx290_write_current_format() --- drivers/media/i2c/imx290.c | 50 ++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index 66cb16f20852..0dc536893ebf 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -152,6 +152,9 @@ #define IMX290_PIXEL_ARRAY_RECORDING_WIDTH 1920 #define IMX290_PIXEL_ARRAY_RECORDING_HEIGHT 1080 +/* Equivalent value for 16bpp */ +#define IMX290_BLACK_LEVEL_DEFAULT 3840 + #define IMX290_NUM_SUPPLIES 3 struct imx290_regval { @@ -315,7 +318,6 @@ static const struct imx290_regval imx290_10bit_settings[] = { { IMX290_ADBIT2, IMX290_ADBIT2_10BIT }, { IMX290_ADBIT3, IMX290_ADBIT3_10BIT }, { IMX290_CSI_DT_FMT, IMX290_CSI_DT_FMT_RAW10 }, - { IMX290_BLKLEVEL, 60 }, }; static const struct imx290_regval imx290_12bit_settings[] = { @@ -325,7 +327,6 @@ static const struct imx290_regval imx290_12bit_settings[] = { { IMX290_ADBIT2, IMX290_ADBIT2_12BIT }, { IMX290_ADBIT3, IMX290_ADBIT3_12BIT }, { IMX290_CSI_DT_FMT, IMX290_CSI_DT_FMT_RAW12 }, - { IMX290_BLKLEVEL, 240 }, }; /* supported link frequencies */ @@ -516,35 +517,40 @@ static int imx290_set_data_lanes(struct imx290 *imx290) return ret; } +static int imx290_set_black_level(struct imx290 *imx290, + unsigned int black_level, int *err) +{ + return imx290_write(imx290, IMX290_BLKLEVEL, + black_level >> (16 - imx290->bpp), err); +} + static int imx290_write_current_format(struct imx290 *imx290) { + const struct imx290_regval *regs; + unsigned int num_regs; int ret; switch (imx290->current_format.code) { case MEDIA_BUS_FMT_SRGGB10_1X10: - ret = imx290_set_register_array(imx290, imx290_10bit_settings, - ARRAY_SIZE( - imx290_10bit_settings)); - if (ret < 0) { - dev_err(imx290->dev, "Could not set format registers\n"); - return ret; - } + regs = imx290_10bit_settings; + num_regs = ARRAY_SIZE(imx290_10bit_settings); break; case MEDIA_BUS_FMT_SRGGB12_1X12: - ret = imx290_set_register_array(imx290, imx290_12bit_settings, - ARRAY_SIZE( - imx290_12bit_settings)); - if (ret < 0) { - dev_err(imx290->dev, "Could not set format registers\n"); - return ret; - } + regs = imx290_12bit_settings; + num_regs = ARRAY_SIZE(imx290_12bit_settings); break; default: dev_err(imx290->dev, "Unknown pixel format\n"); return -EINVAL; } - return 0; + ret = imx290_set_register_array(imx290, regs, num_regs); + if (ret < 0) { + dev_err(imx290->dev, "Could not set format registers\n"); + return ret; + } + + return imx290_set_black_level(imx290, IMX290_BLACK_LEVEL_DEFAULT, &ret); } /* ---------------------------------------------------------------------------- @@ -573,7 +579,7 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl) case V4L2_CID_TEST_PATTERN: if (ctrl->val) { - imx290_write(imx290, IMX290_BLKLEVEL, 0, &ret); + imx290_set_black_level(imx290, 0, &ret); usleep_range(10000, 11000); imx290_write(imx290, IMX290_PGCTRL, (u8)(IMX290_PGCTRL_REGEN | @@ -582,12 +588,8 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl) } else { imx290_write(imx290, IMX290_PGCTRL, 0x00, &ret); usleep_range(10000, 11000); - if (imx290->bpp == 10) - imx290_write(imx290, IMX290_BLKLEVEL, 0x3c, - &ret); - else /* 12 bits per pixel */ - imx290_write(imx290, IMX290_BLKLEVEL, 0xf0, - &ret); + imx290_set_black_level(imx290, IMX290_BLACK_LEVEL_DEFAULT, + &ret); } break; default: From patchwork Sat Jan 14 17:17:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 642821 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 05171C3DA78 for ; Sat, 14 Jan 2023 17:18:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230341AbjANRST (ORCPT ); Sat, 14 Jan 2023 12:18:19 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40542 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230329AbjANRSQ (ORCPT ); Sat, 14 Jan 2023 12:18:16 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5DEABBB85 for ; Sat, 14 Jan 2023 09:18:15 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DC8021752; Sat, 14 Jan 2023 18:18:13 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1673716694; bh=NiUwLp/8TYSvuuqS8g9Tz71O1HCAVwZEQPnHzDz/CFk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hfLMMkXXwF9xCBNGcXK8wfJu0dsCFPQzhzTO9vLGq48E8Nb6gezDl5mBL92iSsv1j V95+nBve4s5NjFoLTBodUhn+A+XKq+Mlxda/Ri+wvmBqEETKTDtnjeCEFp6/na8QWT lVoDuoBbXt+WfEOkg45gzoKh769U2B/H43NDN4LI= From: Laurent Pinchart To: linux-media@vger.kernel.org Cc: Sakari Ailus , Manivannan Sadhasivam , Alexander Stein , Dave Stevenson Subject: [PATCH v2 09/17] media: i2c: imx290: Use dev_err_probe() Date: Sat, 14 Jan 2023 19:17:54 +0200 Message-Id: <20230114171802.13878-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.38.2 In-Reply-To: <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> References: <20230114171727.13830-1-laurent.pinchart@ideasonboard.com> <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Improve error handling in the probe() function with dev_err_probe(). Signed-off-by: Laurent Pinchart Reviewed-by: Alexander Stein --- drivers/media/i2c/imx290.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index 18c1e5c429a2..adccd0662afe 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -1226,10 +1226,9 @@ static int imx290_probe(struct i2c_client *client) /* get system clock (xclk) */ imx290->xclk = devm_clk_get(dev, "xclk"); - if (IS_ERR(imx290->xclk)) { - dev_err(dev, "Could not get xclk"); - return PTR_ERR(imx290->xclk); - } + if (IS_ERR(imx290->xclk)) + return dev_err_probe(dev, PTR_ERR(imx290->xclk), + "Could not get xclk"); ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", &xclk_freq); @@ -1252,17 +1251,14 @@ static int imx290_probe(struct i2c_client *client) } ret = imx290_get_regulators(dev, imx290); - if (ret < 0) { - dev_err(dev, "Cannot get regulators\n"); - return ret; - } + if (ret < 0) + return dev_err_probe(dev, ret, "Cannot get regulators\n"); imx290->rst_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); - if (IS_ERR(imx290->rst_gpio)) { - dev_err(dev, "Cannot get reset gpio\n"); - return PTR_ERR(imx290->rst_gpio); - } + if (IS_ERR(imx290->rst_gpio)) + return dev_err_probe(dev, PTR_ERR(imx290->rst_gpio), + "Cannot get reset gpio\n"); mutex_init(&imx290->lock); From patchwork Sat Jan 14 17:17:56 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 642820 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4A8CAC3DA78 for ; Sat, 14 Jan 2023 17:18:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230352AbjANRSW (ORCPT ); Sat, 14 Jan 2023 12:18:22 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40594 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230336AbjANRST (ORCPT ); Sat, 14 Jan 2023 12:18:19 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 20DA5B774 for ; Sat, 14 Jan 2023 09:18:18 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A281C1A65; Sat, 14 Jan 2023 18:18:16 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1673716696; bh=bkCoF3mLHPyaQhD8c2ySK9YNhR9G+FkI28mftO4X/mk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ngPJLp041AQVg9FpUl/WpPBGnql5kPy6DyXUDCpiLbW9ecEZLszDCjyhCsBq6tofI n02Qal0bFCZ3aqddH6gout0heDPT1OkmLtwo8m43pfQ+uXixYur1NrknGL58HnyiWT CsDQDrk/yjD6Wsnc+4V+pbjANk7fNUokUulsT3u4= From: Laurent Pinchart To: linux-media@vger.kernel.org Cc: Sakari Ailus , Manivannan Sadhasivam , Alexander Stein , Dave Stevenson Subject: [PATCH v2 11/17] media: i2c: imx290: Use V4L2 subdev active state Date: Sat, 14 Jan 2023 19:17:56 +0200 Message-Id: <20230114171802.13878-11-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.38.2 In-Reply-To: <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> References: <20230114171727.13830-1-laurent.pinchart@ideasonboard.com> <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Use the V4L2 subdev active state API to store the active format. This simplifies the driver not only by dropping the imx290 current_format field, but it also allows dropping the imx290 lock, replaced with the state lock. The lock check in imx290_ctrl_update() can be dropped as imx290_set_fmt() can't be called anywmore with which set to ACTIVE before controls are initialized. Signed-off-by: Laurent Pinchart --- Changes since v1: - Remove lock check from imx290_ctrl_update() - Drop unrelated change - Fix error handling in imx290_subdev_init() - Handle errors from v4l2_subdev_init_finalize() - Return immediately from imx290_set_ctrl() for read-only controls - Call v4l2_subdev_cleanup() --- drivers/media/i2c/imx290.c | 170 +++++++++++++++++-------------------- 1 file changed, 80 insertions(+), 90 deletions(-) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index b01a627c8c03..94d23eea36be 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -177,12 +177,12 @@ struct imx290 { struct clk *xclk; struct regmap *regmap; u8 nlanes; - u8 bpp; struct v4l2_subdev sd; struct media_pad pad; - struct v4l2_mbus_framefmt current_format; + const struct imx290_mode *current_mode; + u8 bpp; struct regulator_bulk_data supplies[IMX290_NUM_SUPPLIES]; struct gpio_desc *rst_gpio; @@ -192,8 +192,6 @@ struct imx290 { struct v4l2_ctrl *pixel_rate; struct v4l2_ctrl *hblank; struct v4l2_ctrl *vblank; - - struct mutex lock; }; static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd) @@ -524,13 +522,14 @@ static int imx290_set_black_level(struct imx290 *imx290, black_level >> (16 - imx290->bpp), err); } -static int imx290_write_current_format(struct imx290 *imx290) +static int imx290_setup_format(struct imx290 *imx290, + const struct v4l2_mbus_framefmt *format) { const struct imx290_regval *regs; unsigned int num_regs; int ret; - switch (imx290->current_format.code) { + switch (format->code) { case MEDIA_BUS_FMT_SRGGB10_1X10: regs = imx290_10bit_settings; num_regs = ARRAY_SIZE(imx290_10bit_settings); @@ -561,12 +560,31 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl) { struct imx290 *imx290 = container_of(ctrl->handler, struct imx290, ctrls); + const struct v4l2_mbus_framefmt *format; + struct v4l2_subdev_state *state; int ret = 0; + /* + * Return immediately for controls that don't need to be applied to the + * device. This includes all controls modified in imx290_ctrl_update(), + * which is called at probe time before runtime PM is initialized, so + * we can't proceed to the pm_runtime_get_if_in_use() call below. + */ + switch (ctrl->id) { + case V4L2_CID_LINK_FREQ: + case V4L2_CID_PIXEL_RATE: + case V4L2_CID_VBLANK: + case V4L2_CID_HBLANK: + return 0; + } + /* V4L2 controls values will be applied only when power is already up */ if (!pm_runtime_get_if_in_use(imx290->dev)) return 0; + state = v4l2_subdev_get_locked_active_state(&imx290->sd); + format = v4l2_subdev_get_pad_format(&imx290->sd, state, 0); + switch (ctrl->id) { case V4L2_CID_ANALOGUE_GAIN: ret = imx290_write(imx290, IMX290_GAIN, ctrl->val, NULL); @@ -592,6 +610,7 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl) &ret); } break; + default: ret = -EINVAL; break; @@ -626,13 +645,6 @@ static void imx290_ctrl_update(struct imx290 *imx290, s64 link_freq = imx290_link_freqs_ptr(imx290)[mode->link_freq_index]; u64 pixel_rate; - /* - * This function may be called from imx290_set_fmt() before controls - * get created by imx290_ctrl_init(). Return immediately in that case. - */ - if (!imx290->ctrls.lock) - return; - /* pixel rate = link_freq * 2 * nr_of_lanes / bits_per_sample */ pixel_rate = link_freq * 2 * imx290->nlanes; do_div(pixel_rate, imx290->bpp); @@ -654,7 +666,6 @@ static int imx290_ctrl_init(struct imx290 *imx290) return ret; v4l2_ctrl_handler_init(&imx290->ctrls, 9); - imx290->ctrls.lock = &imx290->lock; /* * The sensor has an analog gain and a digital gain, both controlled @@ -719,11 +730,6 @@ static int imx290_ctrl_init(struct imx290 *imx290) return ret; } - mutex_lock(imx290->ctrls.lock); - imx290_ctrl_update(imx290, &imx290->current_format, - imx290->current_mode); - mutex_unlock(imx290->ctrls.lock); - return 0; } @@ -732,8 +738,10 @@ static int imx290_ctrl_init(struct imx290 *imx290) */ /* Start streaming */ -static int imx290_start_streaming(struct imx290 *imx290) +static int imx290_start_streaming(struct imx290 *imx290, + struct v4l2_subdev_state *state) { + const struct v4l2_mbus_framefmt *format; int ret; /* Set init register settings */ @@ -746,7 +754,8 @@ static int imx290_start_streaming(struct imx290 *imx290) } /* Apply the register values related to current frame format */ - ret = imx290_write_current_format(imx290); + format = v4l2_subdev_get_pad_format(&imx290->sd, state, 0); + ret = imx290_setup_format(imx290, format); if (ret < 0) { dev_err(imx290->dev, "Could not set frame format\n"); return ret; @@ -766,7 +775,7 @@ static int imx290_start_streaming(struct imx290 *imx290) return ret; /* Apply customized values from user */ - ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler); + ret = __v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler); if (ret) { dev_err(imx290->dev, "Could not sync v4l2 controls\n"); return ret; @@ -795,39 +804,32 @@ static int imx290_stop_streaming(struct imx290 *imx290) static int imx290_set_stream(struct v4l2_subdev *sd, int enable) { struct imx290 *imx290 = to_imx290(sd); + struct v4l2_subdev_state *state; int ret = 0; + state = v4l2_subdev_lock_and_get_active_state(sd); + if (enable) { ret = pm_runtime_resume_and_get(imx290->dev); if (ret < 0) - goto unlock_and_return; + goto unlock; - ret = imx290_start_streaming(imx290); + ret = imx290_start_streaming(imx290, state); if (ret) { dev_err(imx290->dev, "Start stream failed\n"); pm_runtime_put(imx290->dev); - goto unlock_and_return; + goto unlock; } } else { imx290_stop_streaming(imx290); pm_runtime_put(imx290->dev); } -unlock_and_return: - +unlock: + v4l2_subdev_unlock_state(state); return ret; } -static struct v4l2_mbus_framefmt * -imx290_get_pad_format(struct imx290 *imx290, struct v4l2_subdev_state *state, - u32 which) -{ - if (which == V4L2_SUBDEV_FORMAT_ACTIVE) - return &imx290->current_format; - else - return v4l2_subdev_get_try_format(&imx290->sd, state, 0); -} - static int imx290_enum_mbus_code(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, struct v4l2_subdev_mbus_code_enum *code) @@ -862,23 +864,6 @@ static int imx290_enum_frame_size(struct v4l2_subdev *sd, return 0; } -static int imx290_get_fmt(struct v4l2_subdev *sd, - struct v4l2_subdev_state *sd_state, - struct v4l2_subdev_format *fmt) -{ - struct imx290 *imx290 = to_imx290(sd); - struct v4l2_mbus_framefmt *framefmt; - - mutex_lock(&imx290->lock); - - framefmt = imx290_get_pad_format(imx290, sd_state, fmt->which); - fmt->format = *framefmt; - - mutex_unlock(&imx290->lock); - - return 0; -} - static int imx290_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, struct v4l2_subdev_format *fmt) @@ -888,8 +873,6 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *format; unsigned int i; - mutex_lock(&imx290->lock); - mode = v4l2_find_nearest_size(imx290_modes_ptr(imx290), imx290_modes_num(imx290), width, height, fmt->format.width, fmt->format.height); @@ -907,7 +890,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, fmt->format.code = imx290_formats[i].code; fmt->format.field = V4L2_FIELD_NONE; - format = imx290_get_pad_format(imx290, sd_state, fmt->which); + format = v4l2_subdev_get_pad_format(sd, sd_state, 0); if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) { imx290->current_mode = mode; @@ -918,8 +901,6 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, *format = fmt->format; - mutex_unlock(&imx290->lock); - return 0; } @@ -927,14 +908,11 @@ static int imx290_get_selection(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, struct v4l2_subdev_selection *sel) { - struct imx290 *imx290 = to_imx290(sd); struct v4l2_mbus_framefmt *format; switch (sel->target) { case V4L2_SEL_TGT_CROP: { - format = imx290_get_pad_format(imx290, sd_state, sel->which); - - mutex_lock(&imx290->lock); + format = v4l2_subdev_get_pad_format(sd, sd_state, 0); sel->r.top = IMX920_PIXEL_ARRAY_MARGIN_TOP + (IMX290_PIXEL_ARRAY_RECORDING_HEIGHT - format->height) / 2; @@ -943,7 +921,6 @@ static int imx290_get_selection(struct v4l2_subdev *sd, sel->r.width = format->width; sel->r.height = format->height; - mutex_unlock(&imx290->lock); return 0; } @@ -972,11 +949,13 @@ static int imx290_get_selection(struct v4l2_subdev *sd, static int imx290_entity_init_cfg(struct v4l2_subdev *subdev, struct v4l2_subdev_state *sd_state) { - struct v4l2_subdev_format fmt = { 0 }; - - fmt.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE; - fmt.format.width = 1920; - fmt.format.height = 1080; + struct v4l2_subdev_format fmt = { + .which = V4L2_SUBDEV_FORMAT_TRY, + .format = { + .width = 1920, + .height = 1080, + }, + }; imx290_set_fmt(subdev, sd_state, &fmt); @@ -991,7 +970,7 @@ static const struct v4l2_subdev_pad_ops imx290_pad_ops = { .init_cfg = imx290_entity_init_cfg, .enum_mbus_code = imx290_enum_mbus_code, .enum_frame_size = imx290_enum_frame_size, - .get_fmt = imx290_get_fmt, + .get_fmt = v4l2_subdev_get_fmt, .set_fmt = imx290_set_fmt, .get_selection = imx290_get_selection, }; @@ -1008,20 +987,12 @@ static const struct media_entity_operations imx290_subdev_entity_ops = { static int imx290_subdev_init(struct imx290 *imx290) { struct i2c_client *client = to_i2c_client(imx290->dev); + const struct v4l2_mbus_framefmt *format; + struct v4l2_subdev_state *state; int ret; - /* - * Initialize the frame format. In particular, imx290->current_mode - * and imx290->bpp are set to defaults: imx290_calc_pixel_rate() call - * below relies on these fields. - */ - imx290_entity_init_cfg(&imx290->sd, NULL); - - ret = imx290_ctrl_init(imx290); - if (ret < 0) { - dev_err(imx290->dev, "Control initialization error %d\n", ret); - return ret; - } + imx290->current_mode = &imx290_modes_ptr(imx290)[0]; + imx290->bpp = imx290_formats[0].bpp; v4l2_i2c_subdev_init(&imx290->sd, client, &imx290_subdev_ops); imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; @@ -1033,15 +1004,40 @@ static int imx290_subdev_init(struct imx290 *imx290) ret = media_entity_pads_init(&imx290->sd.entity, 1, &imx290->pad); if (ret < 0) { dev_err(imx290->dev, "Could not register media entity\n"); - v4l2_ctrl_handler_free(&imx290->ctrls); return ret; } + ret = imx290_ctrl_init(imx290); + if (ret < 0) { + dev_err(imx290->dev, "Control initialization error %d\n", ret); + goto err_media; + } + + imx290->sd.state_lock = imx290->ctrls.lock; + + ret = v4l2_subdev_init_finalize(&imx290->sd); + if (ret < 0) { + dev_err(imx290->dev, "subdev initialization error %d\n", ret); + goto err_ctrls; + } + + state = v4l2_subdev_lock_and_get_active_state(&imx290->sd); + format = v4l2_subdev_get_pad_format(&imx290->sd, state, 0); + imx290_ctrl_update(imx290, format, imx290->current_mode); + v4l2_subdev_unlock_state(state); + return 0; + +err_ctrls: + v4l2_ctrl_handler_free(&imx290->ctrls); +err_media: + media_entity_cleanup(&imx290->sd.entity); + return ret; } static void imx290_subdev_cleanup(struct imx290 *imx290) { + v4l2_subdev_cleanup(&imx290->sd); media_entity_cleanup(&imx290->sd.entity); v4l2_ctrl_handler_free(&imx290->ctrls); } @@ -1272,12 +1268,10 @@ static int imx290_probe(struct i2c_client *client) if (ret) return ret; - mutex_init(&imx290->lock); - /* Initialize and register subdev. */ ret = imx290_subdev_init(imx290); if (ret) - goto err_mutex; + return ret; ret = v4l2_async_register_subdev(&imx290->sd); if (ret < 0) { @@ -1300,8 +1294,6 @@ static int imx290_probe(struct i2c_client *client) err_subdev: imx290_subdev_cleanup(imx290); -err_mutex: - mutex_destroy(&imx290->lock); return ret; } @@ -1314,8 +1306,6 @@ static void imx290_remove(struct i2c_client *client) v4l2_async_unregister_subdev(sd); imx290_subdev_cleanup(imx290); - mutex_destroy(&imx290->lock); - pm_runtime_disable(imx290->dev); if (!pm_runtime_status_suspended(imx290->dev)) imx290_power_off(imx290->dev); From patchwork Sat Jan 14 17:17:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 642819 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 18D04C3DA78 for ; Sat, 14 Jan 2023 17:18:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230345AbjANRS0 (ORCPT ); Sat, 14 Jan 2023 12:18:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40632 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230260AbjANRSV (ORCPT ); Sat, 14 Jan 2023 12:18:21 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E8720B770 for ; Sat, 14 Jan 2023 09:18:20 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 55C1D6DB; Sat, 14 Jan 2023 18:18:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1673716699; bh=VVKiLZpPICLvCW4I5C7KtqcbOsmHlqHCmceSywhYceU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ro4p22kXvo2bM27vLzkaMYHqDzf5Bi5WdnH7QwXxOhLvOZmtXR9TtiAQyfH3QUpQ0 09KhME+RgF2WODlS3ju5gKHKRHI1jRzQKK/8Dgw32U1R3+3lFl53g4inxU4ULuadS9 n9vVaV1PaIi5h+DaCko+wfXJOHGqodk96+O2j7fo= From: Laurent Pinchart To: linux-media@vger.kernel.org Cc: Sakari Ailus , Manivannan Sadhasivam , Alexander Stein , Dave Stevenson Subject: [PATCH v2 13/17] media: i2c: imx290: Use runtime PM autosuspend Date: Sat, 14 Jan 2023 19:17:58 +0200 Message-Id: <20230114171802.13878-13-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.38.2 In-Reply-To: <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> References: <20230114171727.13830-1-laurent.pinchart@ideasonboard.com> <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Use runtime PM autosuspend to avoid powering off the sensor during fast stop-reconfigure-restart cycles. This also fixes runtime PM handling in the probe function that didn't suspend the device, effectively leaving it resumed forever. While at it, improve documentation of power management in probe() and remove(). Signed-off-by: Laurent Pinchart Acked-by: Alexander Stein --- Changes since v1: - Enable autosuspend before registering the subdev - Decrease the PM usage count after registering the subdev - Use pm_runtime_put_autosuspend() in imx290_set_ctrl() --- drivers/media/i2c/imx290.c | 58 +++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index b16fd76e0737..6a3e93c10fb1 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -631,7 +631,8 @@ static int imx290_set_ctrl(struct v4l2_ctrl *ctrl) break; } - pm_runtime_put(imx290->dev); + pm_runtime_mark_last_busy(imx290->dev); + pm_runtime_put_autosuspend(imx290->dev); return ret; } @@ -832,12 +833,13 @@ static int imx290_set_stream(struct v4l2_subdev *sd, int enable) ret = imx290_start_streaming(imx290, state); if (ret) { dev_err(imx290->dev, "Start stream failed\n"); - pm_runtime_put(imx290->dev); + pm_runtime_put_sync(imx290->dev); goto unlock; } } else { imx290_stop_streaming(imx290); - pm_runtime_put(imx290->dev); + pm_runtime_mark_last_busy(imx290->dev); + pm_runtime_put_autosuspend(imx290->dev); } unlock: @@ -1274,33 +1276,59 @@ static int imx290_probe(struct i2c_client *client) if (ret) return ret; - /* Initialize and register subdev. */ + /* Initialize the V4L2 subdev. */ ret = imx290_subdev_init(imx290); if (ret) return ret; - ret = v4l2_async_register_subdev(&imx290->sd); - if (ret < 0) { - dev_err(dev, "Could not register v4l2 device\n"); - goto err_subdev; - } - - /* Power on the device to match runtime PM state below */ + /* + * Enable power management. The driver supports runtime PM, but needs to + * work when runtime PM is disabled in the kernel. To that end, power + * the sensor on manually here. + */ ret = imx290_power_on(dev); if (ret < 0) { dev_err(dev, "Could not power on the device\n"); goto err_subdev; } + /* + * Enable runtime PM with autosuspend. As the device has been powered + * manually, mark it as active, and increase the usage count without + * resuming the device. + */ pm_runtime_set_active(dev); + pm_runtime_get_noresume(dev); pm_runtime_enable(dev); - pm_runtime_idle(dev); + pm_runtime_set_autosuspend_delay(dev, 1000); + pm_runtime_use_autosuspend(dev); + + /* + * Finally, register the V4L2 subdev. This must be done after + * initializing everything as the subdev can be used immediately after + * being registered. + */ + ret = v4l2_async_register_subdev(&imx290->sd); + if (ret < 0) { + dev_err(dev, "Could not register v4l2 device\n"); + goto err_subdev; + } + + /* + * Decrease the PM usage count. The device will get suspended after the + * autosuspend delay, turning the power off. + */ + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); return 0; +err_pm: + pm_runtime_disable(dev); + pm_runtime_put_noidle(dev); + imx290_power_off(dev); err_subdev: imx290_subdev_cleanup(imx290); - return ret; } @@ -1312,6 +1340,10 @@ static void imx290_remove(struct i2c_client *client) v4l2_async_unregister_subdev(sd); imx290_subdev_cleanup(imx290); + /* + * Disable runtime PM. In case runtime PM is disabled in the kernel, + * make sure to turn power off manually. + */ pm_runtime_disable(imx290->dev); if (!pm_runtime_status_suspended(imx290->dev)) imx290_power_off(imx290->dev); From patchwork Sat Jan 14 17:18:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 642818 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F266BC3DA78 for ; Sat, 14 Jan 2023 17:18:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230370AbjANRS3 (ORCPT ); Sat, 14 Jan 2023 12:18:29 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40672 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230359AbjANRSY (ORCPT ); Sat, 14 Jan 2023 12:18:24 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 72429B752 for ; Sat, 14 Jan 2023 09:18:23 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 024984DD; Sat, 14 Jan 2023 18:18:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1673716702; bh=m4eq2QGANKB5k5Q6erW/PQ92at9PuF9FY0y09YvTFfs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jGE8Zeo0LacYMuikeXERa/MPjBNZ97caKjaPrGFGTY4LISZVtNGz2kLihA4Uj7mZA Yu2GWGjwSkNizyTR0Np3mu0XsXMRKJI/s84qTiVEBBzMEce2K9LlrRI5e6OmboASil blQCT/w/9cmHZ2gXBLnhMQWV12jFhhQ23g0RrLIU= From: Laurent Pinchart To: linux-media@vger.kernel.org Cc: Sakari Ailus , Manivannan Sadhasivam , Alexander Stein , Dave Stevenson Subject: [PATCH v2 15/17] media: i2c: imx290: Configure data lanes at start time Date: Sat, 14 Jan 2023 19:18:00 +0200 Message-Id: <20230114171802.13878-15-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.38.2 In-Reply-To: <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> References: <20230114171727.13830-1-laurent.pinchart@ideasonboard.com> <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org There's no need to configure the data lanes in the runtime PM resume handler. Do so in imx290_start_streaming() instead. Signed-off-by: Laurent Pinchart Reviewed-by: Alexander Stein --- drivers/media/i2c/imx290.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index c3ffb23104bf..67a2edd38e91 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -767,6 +767,9 @@ static int imx290_start_streaming(struct imx290 *imx290, return ret; } + /* Set data lane count */ + imx290_set_data_lanes(imx290); + /* Apply the register values related to current frame format */ format = v4l2_subdev_get_pad_format(&imx290->sd, state, 0); ret = imx290_setup_format(imx290, format); @@ -1074,9 +1077,6 @@ static int imx290_power_on(struct imx290 *imx290) gpiod_set_value_cansleep(imx290->rst_gpio, 0); usleep_range(30000, 31000); - /* Set data lane count */ - imx290_set_data_lanes(imx290); - return 0; } From patchwork Sat Jan 14 17:18:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 642817 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9C977C3DA78 for ; Sat, 14 Jan 2023 17:18:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230330AbjANRSd (ORCPT ); Sat, 14 Jan 2023 12:18:33 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40736 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230366AbjANRS2 (ORCPT ); Sat, 14 Jan 2023 12:18:28 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7363AB76B for ; Sat, 14 Jan 2023 09:18:26 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F219F824; Sat, 14 Jan 2023 18:18:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1673716705; bh=UD1YWSW22RUrwYKr9T3h7n/vPt9JRmeaDNvdsMcvqA4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=i58/ciFrN3Ic1IvWTyFl26nG407axYtRZhUvIwniL0qBD0GgCD8j8SamQXzgqiCOr SM61EQpPhsGz8eZRweR+qJAVAx/itH3O2+3SZH2/yGIMRefAJ87zkkrudEULrbMudB YS4/6gs+8dl3h1lGqXHw6gFm0wif4haHcd0KoGQM= From: Laurent Pinchart To: linux-media@vger.kernel.org Cc: Sakari Ailus , Manivannan Sadhasivam , Alexander Stein , Dave Stevenson Subject: [PATCH v2 17/17] media: i2c: imx290: Handle error from imx290_set_data_lanes() Date: Sat, 14 Jan 2023 19:18:02 +0200 Message-Id: <20230114171802.13878-17-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.38.2 In-Reply-To: <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> References: <20230114171727.13830-1-laurent.pinchart@ideasonboard.com> <20230114171802.13878-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Check the error status returned by imx290_set_data_lanes() in its caller and propagate it. Signed-off-by: Laurent Pinchart --- Changes since v1: - New patch --- drivers/media/i2c/imx290.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index a8167119534b..62d8d75aec3e 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -761,7 +761,11 @@ static int imx290_start_streaming(struct imx290 *imx290, } /* Set data lane count */ - imx290_set_data_lanes(imx290); + ret = imx290_set_data_lanes(imx290); + if (ret < 0) { + dev_err(imx290->dev, "Could not set data lanes\n"); + return ret; + } /* Apply the register values related to current frame format */ format = v4l2_subdev_get_pad_format(&imx290->sd, state, 0);