* [PATCH v3 0/3] Conversion of the NOON010PC30 sensor driver to media controller API
@ 2011-09-16 15:59 Sylwester Nawrocki
2011-09-16 15:59 ` [PATCH v3 1/3] noon010pc30: Conversion to the " Sylwester Nawrocki
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Sylwester Nawrocki @ 2011-09-16 15:59 UTC (permalink / raw)
To: linux-media
Cc: m.szyprowski, kyungmin.park, laurent.pinchart, s.nawrocki,
sw0312.kim, riverful.kim
Hello,
The following patch set converts noon010pc30 camera sensor driver to the
subdev pad level operations and user-space V4L2 subdev API.
In addition it implements s_stream operation, removes the now unneeded
g_chip_ident op and tags the driver as experimental.
Changes since v1:
- fixed subdev's flags initialization
- corrected s_stream handler so the sensor's sleep mode is used
for suspending/resuming the output signal
- removed g_chip_indent operation handler
Changes since v2:
- added subdev internal open() operation
- moved registers access to s_power or s_stream so the user space
calls are supported before/without enabling sensor's power
Sylwester Nawrocki (3):
noon010pc30: Conversion to the media controller API
noon010pc30: Improve s_power operation handling
noon010pc30: Remove g_chip_ident operation handler
drivers/media/video/Kconfig | 2 +-
drivers/media/video/noon010pc30.c | 295 ++++++++++++++++++++++---------------
include/media/v4l2-chip-ident.h | 3 -
3 files changed, 177 insertions(+), 123 deletions(-)
Regards,
--
Sylwester Nawrocki
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v3 1/3] noon010pc30: Conversion to the media controller API 2011-09-16 15:59 [PATCH v3 0/3] Conversion of the NOON010PC30 sensor driver to media controller API Sylwester Nawrocki @ 2011-09-16 15:59 ` Sylwester Nawrocki 2011-09-16 16:17 ` Sylwester Nawrocki 2011-09-20 22:18 ` Laurent Pinchart 2011-09-16 15:59 ` [PATCH v3 2/3] noon010pc30: Improve s_power operation handling Sylwester Nawrocki 2011-09-16 15:59 ` [PATCH 3/3 (resend)] noon010pc30: Remove g_chip_ident operation handler Sylwester Nawrocki 2 siblings, 2 replies; 9+ messages in thread From: Sylwester Nawrocki @ 2011-09-16 15:59 UTC (permalink / raw) To: linux-media Cc: m.szyprowski, kyungmin.park, laurent.pinchart, s.nawrocki, sw0312.kim, riverful.kim Replace g/s_mbus_fmt ops with the pad level get/set_fmt operations. Add media entity initialization and set subdev flags so the host driver creates a subdev device node for the driver. A mutex was added for serializing the subdev operations. When setting format is attempted during streaming an (EBUSY) error will be returned. After the device is powered up it will now remain in "power sleep" mode until s_stream(1) is called. The "power sleep" mode is used to suspend/resume frame generation at the sensor's output through s_stream op. While at here simplify the colorspace parameter handling. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- drivers/media/video/Kconfig | 2 +- drivers/media/video/noon010pc30.c | 253 ++++++++++++++++++++++++------------- 2 files changed, 164 insertions(+), 91 deletions(-) diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 6279663..75bb46f 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -755,7 +755,7 @@ config VIDEO_VIA_CAMERA config VIDEO_NOON010PC30 tristate "NOON010PC30 CIF camera sensor support" - depends on I2C && VIDEO_V4L2 + depends on I2C && VIDEO_V4L2 && EXPERIMENTAL && VIDEO_V4L2_SUBDEV_API ---help--- This driver supports NOON010PC30 CIF camera from Siliconfile diff --git a/drivers/media/video/noon010pc30.c b/drivers/media/video/noon010pc30.c index 35f722a..115d976 100644 --- a/drivers/media/video/noon010pc30.c +++ b/drivers/media/video/noon010pc30.c @@ -1,7 +1,7 @@ /* * Driver for SiliconFile NOON010PC30 CIF (1/11") Image Sensor with ISP * - * Copyright (C) 2010 Samsung Electronics + * Copyright (C) 2010 - 2011 Samsung Electronics Co., Ltd. * Contact: Sylwester Nawrocki, <s.nawrocki@samsung.com> * * Initial register configuration based on a driver authored by @@ -10,7 +10,7 @@ * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later vergsion. + * (at your option) any later version. */ #include <linux/delay.h> @@ -113,7 +113,6 @@ MODULE_PARM_DESC(debug, "Enable module debug trace. Set to 1 to enable."); struct noon010_format { enum v4l2_mbus_pixelcode code; - enum v4l2_colorspace colorspace; u16 ispctl1_reg; }; @@ -131,17 +130,24 @@ static const char * const noon010_supply_name[] = { struct noon010_info { struct v4l2_subdev sd; + struct media_pad pad; struct v4l2_ctrl_handler hdl; const struct noon010pc30_platform_data *pdata; + struct regulator_bulk_data supply[NOON010_NUM_SUPPLIES]; + u32 gpio_nreset; + u32 gpio_nstby; + + /* Protects the struct members below */ + struct mutex lock; + const struct noon010_format *curr_fmt; const struct noon010_frmsize *curr_win; + unsigned int apply_new_cfg:1; + unsigned int streaming:1; unsigned int hflip:1; unsigned int vflip:1; unsigned int power:1; u8 i2c_reg_page; - struct regulator_bulk_data supply[NOON010_NUM_SUPPLIES]; - u32 gpio_nreset; - u32 gpio_nstby; }; struct i2c_regval { @@ -168,27 +174,11 @@ static const struct noon010_frmsize noon010_sizes[] = { /* Supported pixel formats. */ static const struct noon010_format noon010_formats[] = { - { - .code = V4L2_MBUS_FMT_YUYV8_2X8, - .colorspace = V4L2_COLORSPACE_JPEG, - .ispctl1_reg = 0x03, - }, { - .code = V4L2_MBUS_FMT_YVYU8_2X8, - .colorspace = V4L2_COLORSPACE_JPEG, - .ispctl1_reg = 0x02, - }, { - .code = V4L2_MBUS_FMT_VYUY8_2X8, - .colorspace = V4L2_COLORSPACE_JPEG, - .ispctl1_reg = 0, - }, { - .code = V4L2_MBUS_FMT_UYVY8_2X8, - .colorspace = V4L2_COLORSPACE_JPEG, - .ispctl1_reg = 0x01, - }, { - .code = V4L2_MBUS_FMT_RGB565_2X8_BE, - .colorspace = V4L2_COLORSPACE_JPEG, - .ispctl1_reg = 0x40, - }, + { V4L2_MBUS_FMT_YUYV8_2X8, 0x03 }, + { V4L2_MBUS_FMT_YVYU8_2X8, 0x02 }, + { V4L2_MBUS_FMT_VYUY8_2X8, 0 }, + { V4L2_MBUS_FMT_UYVY8_2X8, 0x01 }, + { V4L2_MBUS_FMT_RGB565_2X8_BE, 0x40}, }; static const struct i2c_regval noon010_base_regs[] = { @@ -313,6 +303,7 @@ static int noon010_enable_autowhitebalance(struct v4l2_subdev *sd, int on) return ret; } +/* Called with struct noon010_info.lock mutex held */ static int noon010_set_flip(struct v4l2_subdev *sd, int hflip, int vflip) { struct noon010_info *info = to_noon010(sd); @@ -340,21 +331,18 @@ static int noon010_set_flip(struct v4l2_subdev *sd, int hflip, int vflip) static int noon010_set_params(struct v4l2_subdev *sd) { struct noon010_info *info = to_noon010(sd); - int ret; - - if (!info->curr_win) - return -EINVAL; - - ret = cam_i2c_write(sd, VDO_CTL_REG(0), info->curr_win->vid_ctl1); - if (!ret && info->curr_fmt) - ret = cam_i2c_write(sd, ISP_CTL_REG(0), - info->curr_fmt->ispctl1_reg); - return ret; + int ret = cam_i2c_write(sd, VDO_CTL_REG(0), + info->curr_win->vid_ctl1); + if (ret) + return ret; + return cam_i2c_write(sd, ISP_CTL_REG(0), + info->curr_fmt->ispctl1_reg); } /* Find nearest matching image pixel size. */ -static int noon010_try_frame_size(struct v4l2_mbus_framefmt *mf) +static int noon010_try_frame_size(struct v4l2_mbus_framefmt *mf, + const struct noon010_frmsize **size) { unsigned int min_err = ~0; int i = ARRAY_SIZE(noon010_sizes); @@ -374,11 +362,14 @@ static int noon010_try_frame_size(struct v4l2_mbus_framefmt *mf) if (match) { mf->width = match->width; mf->height = match->height; + if (size) + *size = match; return 0; } return -EINVAL; } +/* Called with info.lock mutex held */ static int power_enable(struct noon010_info *info) { int ret; @@ -419,6 +410,7 @@ static int power_enable(struct noon010_info *info) return 0; } +/* Called with info.lock mutex held */ static int power_disable(struct noon010_info *info) { int ret; @@ -448,93 +440,125 @@ static int power_disable(struct noon010_info *info) static int noon010_s_ctrl(struct v4l2_ctrl *ctrl) { struct v4l2_subdev *sd = to_sd(ctrl); + struct noon010_info *info = to_noon010(sd); + int ret = 0; v4l2_dbg(1, debug, sd, "%s: ctrl_id: %d, value: %d\n", __func__, ctrl->id, ctrl->val); + mutex_lock(&info->lock); + /* + * If the device is not powered up by the host driver do + * not apply any controls to H/W at this time. Instead + * the controls will be restored right after power-up. + */ + if (!info->power) + goto unlock; + switch (ctrl->id) { case V4L2_CID_AUTO_WHITE_BALANCE: - return noon010_enable_autowhitebalance(sd, ctrl->val); + ret = noon010_enable_autowhitebalance(sd, ctrl->val); + break; case V4L2_CID_BLUE_BALANCE: - return cam_i2c_write(sd, MWB_BGAIN_REG, ctrl->val); + ret = cam_i2c_write(sd, MWB_BGAIN_REG, ctrl->val); + break; case V4L2_CID_RED_BALANCE: - return cam_i2c_write(sd, MWB_RGAIN_REG, ctrl->val); + ret = cam_i2c_write(sd, MWB_RGAIN_REG, ctrl->val); + break; default: - return -EINVAL; + ret = -EINVAL; } +unlock: + mutex_unlock(&info->lock); + return ret; } -static int noon010_enum_fmt(struct v4l2_subdev *sd, unsigned int index, - enum v4l2_mbus_pixelcode *code) +static int noon010_enum_mbus_code(struct v4l2_subdev *sd, + struct v4l2_subdev_fh *fh, + struct v4l2_subdev_mbus_code_enum *code) { - if (!code || index >= ARRAY_SIZE(noon010_formats)) + if (code->index >= ARRAY_SIZE(noon010_formats)) return -EINVAL; - *code = noon010_formats[index].code; + code->code = noon010_formats[code->index].code; return 0; } -static int noon010_g_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) +static void noon010_get_current_fmt(struct noon010_info *info, + struct v4l2_mbus_framefmt *mf) { - struct noon010_info *info = to_noon010(sd); - int ret; - - if (!mf) - return -EINVAL; - - if (!info->curr_win || !info->curr_fmt) { - ret = noon010_set_params(sd); - if (ret) - return ret; - } - mf->width = info->curr_win->width; mf->height = info->curr_win->height; mf->code = info->curr_fmt->code; - mf->colorspace = info->curr_fmt->colorspace; mf->field = V4L2_FIELD_NONE; + mf->colorspace = V4L2_COLORSPACE_JPEG; +} + +static int noon010_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, + struct v4l2_subdev_format *fmt) +{ + struct noon010_info *info = to_noon010(sd); + struct v4l2_mbus_framefmt *mf; + + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) { + if (fh) { + mf = v4l2_subdev_get_try_format(fh, 0); + fmt->format = *mf; + } + return 0; + } + + mutex_lock(&info->lock); + noon010_get_current_fmt(info, &fmt->format); + mutex_unlock(&info->lock); return 0; } /* Return nearest media bus frame format. */ -static const struct noon010_format *try_fmt(struct v4l2_subdev *sd, +static const struct noon010_format *noon010_try_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) { int i = ARRAY_SIZE(noon010_formats); - noon010_try_frame_size(mf); - - while (i--) + while (--i) if (mf->code == noon010_formats[i].code) break; - mf->code = noon010_formats[i].code; return &noon010_formats[i]; } -static int noon010_try_fmt(struct v4l2_subdev *sd, - struct v4l2_mbus_framefmt *mf) -{ - if (!sd || !mf) - return -EINVAL; - - try_fmt(sd, mf); - return 0; -} - -static int noon010_s_fmt(struct v4l2_subdev *sd, - struct v4l2_mbus_framefmt *mf) +static int noon010_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, + struct v4l2_subdev_format *fmt) { struct noon010_info *info = to_noon010(sd); + const struct noon010_frmsize *size = NULL; + const struct noon010_format *nf; + struct v4l2_mbus_framefmt *mf; + int ret = 0; - if (!sd || !mf) - return -EINVAL; - - info->curr_fmt = try_fmt(sd, mf); + nf = noon010_try_fmt(sd, &fmt->format); + noon010_try_frame_size(&fmt->format, &size); + fmt->format.colorspace = V4L2_COLORSPACE_JPEG; - return noon010_set_params(sd); + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) { + if (fh) { + mf = v4l2_subdev_get_try_format(fh, 0); + *mf = fmt->format; + } + return 0; + } + mutex_lock(&info->lock); + if (!info->streaming) { + info->apply_new_cfg = 1; + info->curr_fmt = nf; + info->curr_win = size; + } else { + ret = -EBUSY; + } + mutex_unlock(&info->lock); + return ret; } static int noon010_base_config(struct v4l2_subdev *sd) @@ -550,8 +574,6 @@ static int noon010_base_config(struct v4l2_subdev *sd) } if (!ret) ret = noon010_set_flip(sd, 1, 0); - if (!ret) - ret = noon010_power_ctrl(sd, false, false); /* sync the handler and the registers state */ v4l2_ctrl_handler_setup(&to_noon010(sd)->hdl); @@ -582,6 +604,26 @@ static int noon010_s_power(struct v4l2_subdev *sd, int on) return ret; } +static int noon010_s_stream(struct v4l2_subdev *sd, int on) +{ + struct noon010_info *info = to_noon010(sd); + int ret = 0; + + mutex_lock(&info->lock); + if (!info->streaming != !on) { + ret = noon010_power_ctrl(sd, false, !on); + if (!ret) + info->streaming = on; + } + if (!ret && on && info->apply_new_cfg) { + ret = noon010_set_params(sd); + if (!ret) + info->apply_new_cfg = 0; + } + mutex_unlock(&info->lock); + return ret; +} + static int noon010_g_chip_ident(struct v4l2_subdev *sd, struct v4l2_dbg_chip_ident *chip) { @@ -599,6 +641,22 @@ static int noon010_log_status(struct v4l2_subdev *sd) return 0; } +static int noon010_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) +{ + struct v4l2_mbus_framefmt *mf = v4l2_subdev_get_try_format(fh, 0); + struct noon010_info *info = to_noon010(sd); + + mutex_lock(&info->lock); + noon010_get_current_fmt(to_noon010(sd), mf); + + mutex_unlock(&info->lock); + return 0; +} + +static const struct v4l2_subdev_internal_ops noon010_subdev_internal_ops = { + .open = noon010_open, +}; + static const struct v4l2_ctrl_ops noon010_ctrl_ops = { .s_ctrl = noon010_s_ctrl, }; @@ -616,15 +674,19 @@ static const struct v4l2_subdev_core_ops noon010_core_ops = { .log_status = noon010_log_status, }; -static const struct v4l2_subdev_video_ops noon010_video_ops = { - .g_mbus_fmt = noon010_g_fmt, - .s_mbus_fmt = noon010_s_fmt, - .try_mbus_fmt = noon010_try_fmt, - .enum_mbus_fmt = noon010_enum_fmt, +static struct v4l2_subdev_pad_ops noon010_pad_ops = { + .enum_mbus_code = noon010_enum_mbus_code, + .get_fmt = noon010_get_fmt, + .set_fmt = noon010_set_fmt, +}; + +static struct v4l2_subdev_video_ops noon010_video_ops = { + .s_stream = noon010_s_stream, }; static const struct v4l2_subdev_ops noon010_ops = { .core = &noon010_core_ops, + .pad = &noon010_pad_ops, .video = &noon010_video_ops, }; @@ -665,10 +727,14 @@ static int noon010_probe(struct i2c_client *client, if (!info) return -ENOMEM; + mutex_init(&info->lock); sd = &info->sd; strlcpy(sd->name, MODULE_NAME, sizeof(sd->name)); v4l2_i2c_subdev_init(sd, client, &noon010_ops); + sd->internal_ops = &noon010_subdev_internal_ops; + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + v4l2_ctrl_handler_init(&info->hdl, 3); v4l2_ctrl_new_std(&info->hdl, &noon010_ctrl_ops, @@ -719,11 +785,17 @@ static int noon010_probe(struct i2c_client *client, if (ret) goto np_reg_err; + info->pad.flags = MEDIA_PAD_FL_SOURCE; + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR; + ret = media_entity_init(&sd->entity, 1, &info->pad, 0); + if (ret < 0) + goto np_me_err; + ret = noon010_detect(client, info); if (!ret) return 0; - /* the sensor detection failed */ +np_me_err: regulator_bulk_free(NOON010_NUM_SUPPLIES, info->supply); np_reg_err: if (gpio_is_valid(info->gpio_nstby)) @@ -754,6 +826,7 @@ static int noon010_remove(struct i2c_client *client) if (gpio_is_valid(info->gpio_nstby)) gpio_free(info->gpio_nstby); + media_entity_cleanup(&sd->entity); kfree(info); return 0; } -- 1.7.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 1/3] noon010pc30: Conversion to the media controller API 2011-09-16 15:59 ` [PATCH v3 1/3] noon010pc30: Conversion to the " Sylwester Nawrocki @ 2011-09-16 16:17 ` Sylwester Nawrocki 2011-09-20 22:18 ` Laurent Pinchart 1 sibling, 0 replies; 9+ messages in thread From: Sylwester Nawrocki @ 2011-09-16 16:17 UTC (permalink / raw) To: linux-media; +Cc: kyungmin.park, m.szyprowski, Sylwester Nawrocki Replace g/s_mbus_fmt ops with the pad level get/set_fmt operations. Add media entity initialization and set subdev flags so the host driver creates a subdev device node for the driver. A mutex was added for serializing the subdev operations. When setting format is attempted during streaming an (EBUSY) error will be returned. After the device is powered up it will now remain in "power sleep" mode until s_stream(1) is called. The "power sleep" mode is used to suspend/resume frame generation at the sensor's output through s_stream op. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- This is just a resend as I just previously sent wrong version. Sorry about the mistake. --- drivers/media/video/Kconfig | 2 +- drivers/media/video/noon010pc30.c | 224 ++++++++++++++++++++++++++----------- 2 files changed, 158 insertions(+), 68 deletions(-) diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 6279663..75bb46f 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -755,7 +755,7 @@ config VIDEO_VIA_CAMERA config VIDEO_NOON010PC30 tristate "NOON010PC30 CIF camera sensor support" - depends on I2C && VIDEO_V4L2 + depends on I2C && VIDEO_V4L2 && EXPERIMENTAL && VIDEO_V4L2_SUBDEV_API ---help--- This driver supports NOON010PC30 CIF camera from Siliconfile diff --git a/drivers/media/video/noon010pc30.c b/drivers/media/video/noon010pc30.c index 35f722a..f91ae14 100644 --- a/drivers/media/video/noon010pc30.c +++ b/drivers/media/video/noon010pc30.c @@ -1,7 +1,7 @@ /* * Driver for SiliconFile NOON010PC30 CIF (1/11") Image Sensor with ISP * - * Copyright (C) 2010 Samsung Electronics + * Copyright (C) 2010 - 2011 Samsung Electronics Co., Ltd. * Contact: Sylwester Nawrocki, <s.nawrocki@samsung.com> * * Initial register configuration based on a driver authored by @@ -10,7 +10,7 @@ * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later vergsion. + * (at your option) any later version. */ #include <linux/delay.h> @@ -131,17 +131,24 @@ static const char * const noon010_supply_name[] = { struct noon010_info { struct v4l2_subdev sd; + struct media_pad pad; struct v4l2_ctrl_handler hdl; const struct noon010pc30_platform_data *pdata; + struct regulator_bulk_data supply[NOON010_NUM_SUPPLIES]; + u32 gpio_nreset; + u32 gpio_nstby; + + /* Protects the struct members below */ + struct mutex lock; + const struct noon010_format *curr_fmt; const struct noon010_frmsize *curr_win; + unsigned int apply_new_cfg:1; + unsigned int streaming:1; unsigned int hflip:1; unsigned int vflip:1; unsigned int power:1; u8 i2c_reg_page; - struct regulator_bulk_data supply[NOON010_NUM_SUPPLIES]; - u32 gpio_nreset; - u32 gpio_nstby; }; struct i2c_regval { @@ -313,6 +320,7 @@ static int noon010_enable_autowhitebalance(struct v4l2_subdev *sd, int on) return ret; } +/* Called with struct noon010_info.lock mutex held */ static int noon010_set_flip(struct v4l2_subdev *sd, int hflip, int vflip) { struct noon010_info *info = to_noon010(sd); @@ -340,21 +348,18 @@ static int noon010_set_flip(struct v4l2_subdev *sd, int hflip, int vflip) static int noon010_set_params(struct v4l2_subdev *sd) { struct noon010_info *info = to_noon010(sd); - int ret; - - if (!info->curr_win) - return -EINVAL; - ret = cam_i2c_write(sd, VDO_CTL_REG(0), info->curr_win->vid_ctl1); - - if (!ret && info->curr_fmt) - ret = cam_i2c_write(sd, ISP_CTL_REG(0), - info->curr_fmt->ispctl1_reg); - return ret; + int ret = cam_i2c_write(sd, VDO_CTL_REG(0), + info->curr_win->vid_ctl1); + if (ret) + return ret; + return cam_i2c_write(sd, ISP_CTL_REG(0), + info->curr_fmt->ispctl1_reg); } /* Find nearest matching image pixel size. */ -static int noon010_try_frame_size(struct v4l2_mbus_framefmt *mf) +static int noon010_try_frame_size(struct v4l2_mbus_framefmt *mf, + const struct noon010_frmsize **size) { unsigned int min_err = ~0; int i = ARRAY_SIZE(noon010_sizes); @@ -374,11 +379,14 @@ static int noon010_try_frame_size(struct v4l2_mbus_framefmt *mf) if (match) { mf->width = match->width; mf->height = match->height; + if (size) + *size = match; return 0; } return -EINVAL; } +/* Called with info.lock mutex held */ static int power_enable(struct noon010_info *info) { int ret; @@ -419,6 +427,7 @@ static int power_enable(struct noon010_info *info) return 0; } +/* Called with info.lock mutex held */ static int power_disable(struct noon010_info *info) { int ret; @@ -448,93 +457,125 @@ static int power_disable(struct noon010_info *info) static int noon010_s_ctrl(struct v4l2_ctrl *ctrl) { struct v4l2_subdev *sd = to_sd(ctrl); + struct noon010_info *info = to_noon010(sd); + int ret = 0; v4l2_dbg(1, debug, sd, "%s: ctrl_id: %d, value: %d\n", __func__, ctrl->id, ctrl->val); + mutex_lock(&info->lock); + /* + * If the device is not powered up by the host driver do + * not apply any controls to H/W at this time. Instead + * the controls will be restored right after power-up. + */ + if (!info->power) + goto unlock; + switch (ctrl->id) { case V4L2_CID_AUTO_WHITE_BALANCE: - return noon010_enable_autowhitebalance(sd, ctrl->val); + ret = noon010_enable_autowhitebalance(sd, ctrl->val); + break; case V4L2_CID_BLUE_BALANCE: - return cam_i2c_write(sd, MWB_BGAIN_REG, ctrl->val); + ret = cam_i2c_write(sd, MWB_BGAIN_REG, ctrl->val); + break; case V4L2_CID_RED_BALANCE: - return cam_i2c_write(sd, MWB_RGAIN_REG, ctrl->val); + ret = cam_i2c_write(sd, MWB_RGAIN_REG, ctrl->val); + break; default: - return -EINVAL; + ret = -EINVAL; } +unlock: + mutex_unlock(&info->lock); + return ret; } -static int noon010_enum_fmt(struct v4l2_subdev *sd, unsigned int index, - enum v4l2_mbus_pixelcode *code) +static int noon010_enum_mbus_code(struct v4l2_subdev *sd, + struct v4l2_subdev_fh *fh, + struct v4l2_subdev_mbus_code_enum *code) { - if (!code || index >= ARRAY_SIZE(noon010_formats)) + if (code->index >= ARRAY_SIZE(noon010_formats)) return -EINVAL; - *code = noon010_formats[index].code; + code->code = noon010_formats[code->index].code; return 0; } -static int noon010_g_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) +static void noon010_get_current_fmt(struct noon010_info *info, + struct v4l2_mbus_framefmt *mf) { - struct noon010_info *info = to_noon010(sd); - int ret; - - if (!mf) - return -EINVAL; - - if (!info->curr_win || !info->curr_fmt) { - ret = noon010_set_params(sd); - if (ret) - return ret; - } - mf->width = info->curr_win->width; mf->height = info->curr_win->height; mf->code = info->curr_fmt->code; mf->colorspace = info->curr_fmt->colorspace; mf->field = V4L2_FIELD_NONE; +} + +static int noon010_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, + struct v4l2_subdev_format *fmt) +{ + struct noon010_info *info = to_noon010(sd); + struct v4l2_mbus_framefmt *mf; + + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) { + if (fh) { + mf = v4l2_subdev_get_try_format(fh, 0); + fmt->format = *mf; + } + return 0; + } + + mutex_lock(&info->lock); + noon010_get_current_fmt(info, &fmt->format); + mutex_unlock(&info->lock); return 0; } /* Return nearest media bus frame format. */ -static const struct noon010_format *try_fmt(struct v4l2_subdev *sd, +static const struct noon010_format *noon010_try_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) { int i = ARRAY_SIZE(noon010_formats); - noon010_try_frame_size(mf); - - while (i--) + while (--i) if (mf->code == noon010_formats[i].code) break; - mf->code = noon010_formats[i].code; return &noon010_formats[i]; } -static int noon010_try_fmt(struct v4l2_subdev *sd, - struct v4l2_mbus_framefmt *mf) -{ - if (!sd || !mf) - return -EINVAL; - - try_fmt(sd, mf); - return 0; -} - -static int noon010_s_fmt(struct v4l2_subdev *sd, - struct v4l2_mbus_framefmt *mf) +static int noon010_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, + struct v4l2_subdev_format *fmt) { struct noon010_info *info = to_noon010(sd); + const struct noon010_frmsize *size = NULL; + const struct noon010_format *nf; + struct v4l2_mbus_framefmt *mf; + int ret = 0; - if (!sd || !mf) - return -EINVAL; - - info->curr_fmt = try_fmt(sd, mf); + nf = noon010_try_fmt(sd, &fmt->format); + noon010_try_frame_size(&fmt->format, &size); + fmt->format.colorspace = V4L2_COLORSPACE_JPEG; - return noon010_set_params(sd); + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) { + if (fh) { + mf = v4l2_subdev_get_try_format(fh, 0); + *mf = fmt->format; + } + return 0; + } + mutex_lock(&info->lock); + if (!info->streaming) { + info->apply_new_cfg = 1; + info->curr_fmt = nf; + info->curr_win = size; + } else { + ret = -EBUSY; + } + mutex_unlock(&info->lock); + return ret; } static int noon010_base_config(struct v4l2_subdev *sd) @@ -550,8 +591,6 @@ static int noon010_base_config(struct v4l2_subdev *sd) } if (!ret) ret = noon010_set_flip(sd, 1, 0); - if (!ret) - ret = noon010_power_ctrl(sd, false, false); /* sync the handler and the registers state */ v4l2_ctrl_handler_setup(&to_noon010(sd)->hdl); @@ -582,6 +621,26 @@ static int noon010_s_power(struct v4l2_subdev *sd, int on) return ret; } +static int noon010_s_stream(struct v4l2_subdev *sd, int on) +{ + struct noon010_info *info = to_noon010(sd); + int ret = 0; + + mutex_lock(&info->lock); + if (!info->streaming != !on) { + ret = noon010_power_ctrl(sd, false, !on); + if (!ret) + info->streaming = on; + } + if (!ret && on && info->apply_new_cfg) { + ret = noon010_set_params(sd); + if (!ret) + info->apply_new_cfg = 0; + } + mutex_unlock(&info->lock); + return ret; +} + static int noon010_g_chip_ident(struct v4l2_subdev *sd, struct v4l2_dbg_chip_ident *chip) { @@ -599,6 +658,22 @@ static int noon010_log_status(struct v4l2_subdev *sd) return 0; } +static int noon010_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) +{ + struct v4l2_mbus_framefmt *mf = v4l2_subdev_get_try_format(fh, 0); + struct noon010_info *info = to_noon010(sd); + + mutex_lock(&info->lock); + noon010_get_current_fmt(to_noon010(sd), mf); + + mutex_unlock(&info->lock); + return 0; +} + +static const struct v4l2_subdev_internal_ops noon010_subdev_internal_ops = { + .open = noon010_open, +}; + static const struct v4l2_ctrl_ops noon010_ctrl_ops = { .s_ctrl = noon010_s_ctrl, }; @@ -616,15 +691,19 @@ static const struct v4l2_subdev_core_ops noon010_core_ops = { .log_status = noon010_log_status, }; -static const struct v4l2_subdev_video_ops noon010_video_ops = { - .g_mbus_fmt = noon010_g_fmt, - .s_mbus_fmt = noon010_s_fmt, - .try_mbus_fmt = noon010_try_fmt, - .enum_mbus_fmt = noon010_enum_fmt, +static struct v4l2_subdev_pad_ops noon010_pad_ops = { + .enum_mbus_code = noon010_enum_mbus_code, + .get_fmt = noon010_get_fmt, + .set_fmt = noon010_set_fmt, +}; + +static struct v4l2_subdev_video_ops noon010_video_ops = { + .s_stream = noon010_s_stream, }; static const struct v4l2_subdev_ops noon010_ops = { .core = &noon010_core_ops, + .pad = &noon010_pad_ops, .video = &noon010_video_ops, }; @@ -665,10 +744,14 @@ static int noon010_probe(struct i2c_client *client, if (!info) return -ENOMEM; + mutex_init(&info->lock); sd = &info->sd; strlcpy(sd->name, MODULE_NAME, sizeof(sd->name)); v4l2_i2c_subdev_init(sd, client, &noon010_ops); + sd->internal_ops = &noon010_subdev_internal_ops; + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + v4l2_ctrl_handler_init(&info->hdl, 3); v4l2_ctrl_new_std(&info->hdl, &noon010_ctrl_ops, @@ -719,11 +802,17 @@ static int noon010_probe(struct i2c_client *client, if (ret) goto np_reg_err; + info->pad.flags = MEDIA_PAD_FL_SOURCE; + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR; + ret = media_entity_init(&sd->entity, 1, &info->pad, 0); + if (ret < 0) + goto np_me_err; + ret = noon010_detect(client, info); if (!ret) return 0; - /* the sensor detection failed */ +np_me_err: regulator_bulk_free(NOON010_NUM_SUPPLIES, info->supply); np_reg_err: if (gpio_is_valid(info->gpio_nstby)) @@ -754,6 +843,7 @@ static int noon010_remove(struct i2c_client *client) if (gpio_is_valid(info->gpio_nstby)) gpio_free(info->gpio_nstby); + media_entity_cleanup(&sd->entity); kfree(info); return 0; } -- 1.7.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] noon010pc30: Conversion to the media controller API 2011-09-16 15:59 ` [PATCH v3 1/3] noon010pc30: Conversion to the " Sylwester Nawrocki 2011-09-16 16:17 ` Sylwester Nawrocki @ 2011-09-20 22:18 ` Laurent Pinchart 2011-09-21 13:54 ` Sylwester Nawrocki 2011-09-21 14:26 ` [PATCH v4] " Sylwester Nawrocki 1 sibling, 2 replies; 9+ messages in thread From: Laurent Pinchart @ 2011-09-20 22:18 UTC (permalink / raw) To: Sylwester Nawrocki Cc: linux-media, m.szyprowski, kyungmin.park, sw0312.kim, riverful.kim Hi Sylwester, Thanks for the patch. On Friday 16 September 2011 17:59:54 Sylwester Nawrocki wrote: > Replace g/s_mbus_fmt ops with the pad level get/set_fmt operations. > Add media entity initialization and set subdev flags so the host driver > creates a subdev device node for the driver. > A mutex was added for serializing the subdev operations. When setting > format is attempted during streaming an (EBUSY) error will be returned. > > After the device is powered up it will now remain in "power sleep" > mode until s_stream(1) is called. The "power sleep" mode is used > to suspend/resume frame generation at the sensor's output through > s_stream op. > > While at here simplify the colorspace parameter handling. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> [snip] > diff --git a/drivers/media/video/noon010pc30.c > b/drivers/media/video/noon010pc30.c index 35f722a..115d976 100644 > --- a/drivers/media/video/noon010pc30.c > +++ b/drivers/media/video/noon010pc30.c [snip] > @@ -599,6 +641,22 @@ static int noon010_log_status(struct v4l2_subdev *sd) > return 0; > } > > +static int noon010_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) > +{ > + struct v4l2_mbus_framefmt *mf = v4l2_subdev_get_try_format(fh, 0); > + struct noon010_info *info = to_noon010(sd); > + > + mutex_lock(&info->lock); > + noon010_get_current_fmt(to_noon010(sd), mf); Should you initialize mf with a constant default format instead of retrieving the current format from the sensor ? A non-constant default would probably confuse userspace application. > + > + mutex_unlock(&info->lock); > + return 0; > +} -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] noon010pc30: Conversion to the media controller API 2011-09-20 22:18 ` Laurent Pinchart @ 2011-09-21 13:54 ` Sylwester Nawrocki 2011-09-21 14:26 ` [PATCH v4] " Sylwester Nawrocki 1 sibling, 0 replies; 9+ messages in thread From: Sylwester Nawrocki @ 2011-09-21 13:54 UTC (permalink / raw) To: Laurent Pinchart Cc: linux-media, m.szyprowski, kyungmin.park, sw0312.kim, riverful.kim Hi Laurent, thanks for the review. On 09/21/2011 12:18 AM, Laurent Pinchart wrote: > Hi Sylwester, > > Thanks for the patch. > > On Friday 16 September 2011 17:59:54 Sylwester Nawrocki wrote: >> Replace g/s_mbus_fmt ops with the pad level get/set_fmt operations. >> Add media entity initialization and set subdev flags so the host driver >> creates a subdev device node for the driver. >> A mutex was added for serializing the subdev operations. When setting >> format is attempted during streaming an (EBUSY) error will be returned. >> >> After the device is powered up it will now remain in "power sleep" >> mode until s_stream(1) is called. The "power sleep" mode is used >> to suspend/resume frame generation at the sensor's output through >> s_stream op. >> >> While at here simplify the colorspace parameter handling. >> >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > > [snip] > >> diff --git a/drivers/media/video/noon010pc30.c >> b/drivers/media/video/noon010pc30.c index 35f722a..115d976 100644 >> --- a/drivers/media/video/noon010pc30.c >> +++ b/drivers/media/video/noon010pc30.c > > [snip] > >> @@ -599,6 +641,22 @@ static int noon010_log_status(struct v4l2_subdev *sd) >> return 0; >> } >> >> +static int noon010_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) >> +{ >> + struct v4l2_mbus_framefmt *mf = v4l2_subdev_get_try_format(fh, 0); >> + struct noon010_info *info = to_noon010(sd); >> + >> + mutex_lock(&info->lock); >> + noon010_get_current_fmt(to_noon010(sd), mf); > > Should you initialize mf with a constant default format instead of retrieving > the current format from the sensor ? A non-constant default would probably > confuse userspace application. Sure, I could change to it. But I don't quite see the problem, the applications should call set_fmt(TRY) anyway, rather than relying on the format gotten right after opening the device, shouldn't they ? Anyway I guess it's better to have all drivers behaving consistently. Regards, -- Sylwester Nawrocki Samsung Poland R&D Center ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4] noon010pc30: Conversion to the media controller API 2011-09-20 22:18 ` Laurent Pinchart 2011-09-21 13:54 ` Sylwester Nawrocki @ 2011-09-21 14:26 ` Sylwester Nawrocki 2011-09-21 14:28 ` Laurent Pinchart 1 sibling, 1 reply; 9+ messages in thread From: Sylwester Nawrocki @ 2011-09-21 14:26 UTC (permalink / raw) To: linux-media Cc: kyungmin.park, m.szyprowski, laurent.pinchart, Sylwester Nawrocki Replace g/s_mbus_fmt ops with the pad level get/set_fmt operations. Add media entity initialization and set subdev flags so the host driver creates a subdev device node for the driver. A mutex was added for serializing the subdev operations. When setting format is attempted during streaming an (EBUSY) error will be returned. After the device is powered up it will now remain in "power sleep" mode until s_stream(1) is called. The "power sleep" mode is used to suspend/resume frame generation at the sensor's output through s_stream op. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- Addresing Laurent's comments, changes since v3: - set constant default TRY format in subdev open() internal op rather than the currently set format --- drivers/media/video/Kconfig | 2 +- drivers/media/video/noon010pc30.c | 221 +++++++++++++++++++++++++----------- 2 files changed, 154 insertions(+), 69 deletions(-) diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 6279663..75bb46f 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -755,7 +755,7 @@ config VIDEO_VIA_CAMERA config VIDEO_NOON010PC30 tristate "NOON010PC30 CIF camera sensor support" - depends on I2C && VIDEO_V4L2 + depends on I2C && VIDEO_V4L2 && EXPERIMENTAL && VIDEO_V4L2_SUBDEV_API ---help--- This driver supports NOON010PC30 CIF camera from Siliconfile diff --git a/drivers/media/video/noon010pc30.c b/drivers/media/video/noon010pc30.c index 35f722a..1a874ec 100644 --- a/drivers/media/video/noon010pc30.c +++ b/drivers/media/video/noon010pc30.c @@ -1,7 +1,7 @@ /* * Driver for SiliconFile NOON010PC30 CIF (1/11") Image Sensor with ISP * - * Copyright (C) 2010 Samsung Electronics + * Copyright (C) 2010 - 2011 Samsung Electronics Co., Ltd. * Contact: Sylwester Nawrocki, <s.nawrocki@samsung.com> * * Initial register configuration based on a driver authored by @@ -10,7 +10,7 @@ * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later vergsion. + * (at your option) any later version. */ #include <linux/delay.h> @@ -131,17 +131,24 @@ static const char * const noon010_supply_name[] = { struct noon010_info { struct v4l2_subdev sd; + struct media_pad pad; struct v4l2_ctrl_handler hdl; const struct noon010pc30_platform_data *pdata; + struct regulator_bulk_data supply[NOON010_NUM_SUPPLIES]; + u32 gpio_nreset; + u32 gpio_nstby; + + /* Protects the struct members below */ + struct mutex lock; + const struct noon010_format *curr_fmt; const struct noon010_frmsize *curr_win; + unsigned int apply_new_cfg:1; + unsigned int streaming:1; unsigned int hflip:1; unsigned int vflip:1; unsigned int power:1; u8 i2c_reg_page; - struct regulator_bulk_data supply[NOON010_NUM_SUPPLIES]; - u32 gpio_nreset; - u32 gpio_nstby; }; struct i2c_regval { @@ -313,6 +320,7 @@ static int noon010_enable_autowhitebalance(struct v4l2_subdev *sd, int on) return ret; } +/* Called with struct noon010_info.lock mutex held */ static int noon010_set_flip(struct v4l2_subdev *sd, int hflip, int vflip) { struct noon010_info *info = to_noon010(sd); @@ -340,21 +348,18 @@ static int noon010_set_flip(struct v4l2_subdev *sd, int hflip, int vflip) static int noon010_set_params(struct v4l2_subdev *sd) { struct noon010_info *info = to_noon010(sd); - int ret; - if (!info->curr_win) - return -EINVAL; - - ret = cam_i2c_write(sd, VDO_CTL_REG(0), info->curr_win->vid_ctl1); - - if (!ret && info->curr_fmt) - ret = cam_i2c_write(sd, ISP_CTL_REG(0), - info->curr_fmt->ispctl1_reg); - return ret; + int ret = cam_i2c_write(sd, VDO_CTL_REG(0), + info->curr_win->vid_ctl1); + if (ret) + return ret; + return cam_i2c_write(sd, ISP_CTL_REG(0), + info->curr_fmt->ispctl1_reg); } /* Find nearest matching image pixel size. */ -static int noon010_try_frame_size(struct v4l2_mbus_framefmt *mf) +static int noon010_try_frame_size(struct v4l2_mbus_framefmt *mf, + const struct noon010_frmsize **size) { unsigned int min_err = ~0; int i = ARRAY_SIZE(noon010_sizes); @@ -374,11 +379,14 @@ static int noon010_try_frame_size(struct v4l2_mbus_framefmt *mf) if (match) { mf->width = match->width; mf->height = match->height; + if (size) + *size = match; return 0; } return -EINVAL; } +/* Called with info.lock mutex held */ static int power_enable(struct noon010_info *info) { int ret; @@ -419,6 +427,7 @@ static int power_enable(struct noon010_info *info) return 0; } +/* Called with info.lock mutex held */ static int power_disable(struct noon010_info *info) { int ret; @@ -448,93 +457,120 @@ static int power_disable(struct noon010_info *info) static int noon010_s_ctrl(struct v4l2_ctrl *ctrl) { struct v4l2_subdev *sd = to_sd(ctrl); + struct noon010_info *info = to_noon010(sd); + int ret = 0; v4l2_dbg(1, debug, sd, "%s: ctrl_id: %d, value: %d\n", __func__, ctrl->id, ctrl->val); + mutex_lock(&info->lock); + /* + * If the device is not powered up by the host driver do + * not apply any controls to H/W at this time. Instead + * the controls will be restored right after power-up. + */ + if (!info->power) + goto unlock; + switch (ctrl->id) { case V4L2_CID_AUTO_WHITE_BALANCE: - return noon010_enable_autowhitebalance(sd, ctrl->val); + ret = noon010_enable_autowhitebalance(sd, ctrl->val); + break; case V4L2_CID_BLUE_BALANCE: - return cam_i2c_write(sd, MWB_BGAIN_REG, ctrl->val); + ret = cam_i2c_write(sd, MWB_BGAIN_REG, ctrl->val); + break; case V4L2_CID_RED_BALANCE: - return cam_i2c_write(sd, MWB_RGAIN_REG, ctrl->val); + ret = cam_i2c_write(sd, MWB_RGAIN_REG, ctrl->val); + break; default: - return -EINVAL; + ret = -EINVAL; } +unlock: + mutex_unlock(&info->lock); + return ret; } -static int noon010_enum_fmt(struct v4l2_subdev *sd, unsigned int index, - enum v4l2_mbus_pixelcode *code) +static int noon010_enum_mbus_code(struct v4l2_subdev *sd, + struct v4l2_subdev_fh *fh, + struct v4l2_subdev_mbus_code_enum *code) { - if (!code || index >= ARRAY_SIZE(noon010_formats)) + if (code->index >= ARRAY_SIZE(noon010_formats)) return -EINVAL; - *code = noon010_formats[index].code; + code->code = noon010_formats[code->index].code; return 0; } -static int noon010_g_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) +static int noon010_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, + struct v4l2_subdev_format *fmt) { struct noon010_info *info = to_noon010(sd); - int ret; - - if (!mf) - return -EINVAL; + struct v4l2_mbus_framefmt *mf; - if (!info->curr_win || !info->curr_fmt) { - ret = noon010_set_params(sd); - if (ret) - return ret; + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) { + if (fh) { + mf = v4l2_subdev_get_try_format(fh, 0); + fmt->format = *mf; + } + return 0; } + mf = &fmt->format; - mf->width = info->curr_win->width; - mf->height = info->curr_win->height; - mf->code = info->curr_fmt->code; - mf->colorspace = info->curr_fmt->colorspace; - mf->field = V4L2_FIELD_NONE; + mutex_lock(&info->lock); + mf->width = info->curr_win->width; + mf->height = info->curr_win->height; + mf->code = info->curr_fmt->code; + mf->colorspace = info->curr_fmt->colorspace; + mf->field = V4L2_FIELD_NONE; + mutex_unlock(&info->lock); return 0; } /* Return nearest media bus frame format. */ -static const struct noon010_format *try_fmt(struct v4l2_subdev *sd, +static const struct noon010_format *noon010_try_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) { int i = ARRAY_SIZE(noon010_formats); - noon010_try_frame_size(mf); - - while (i--) + while (--i) if (mf->code == noon010_formats[i].code) break; - mf->code = noon010_formats[i].code; return &noon010_formats[i]; } -static int noon010_try_fmt(struct v4l2_subdev *sd, - struct v4l2_mbus_framefmt *mf) -{ - if (!sd || !mf) - return -EINVAL; - - try_fmt(sd, mf); - return 0; -} - -static int noon010_s_fmt(struct v4l2_subdev *sd, - struct v4l2_mbus_framefmt *mf) +static int noon010_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, + struct v4l2_subdev_format *fmt) { struct noon010_info *info = to_noon010(sd); + const struct noon010_frmsize *size = NULL; + const struct noon010_format *nf; + struct v4l2_mbus_framefmt *mf; + int ret = 0; - if (!sd || !mf) - return -EINVAL; - - info->curr_fmt = try_fmt(sd, mf); + nf = noon010_try_fmt(sd, &fmt->format); + noon010_try_frame_size(&fmt->format, &size); + fmt->format.colorspace = V4L2_COLORSPACE_JPEG; - return noon010_set_params(sd); + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) { + if (fh) { + mf = v4l2_subdev_get_try_format(fh, 0); + *mf = fmt->format; + } + return 0; + } + mutex_lock(&info->lock); + if (!info->streaming) { + info->apply_new_cfg = 1; + info->curr_fmt = nf; + info->curr_win = size; + } else { + ret = -EBUSY; + } + mutex_unlock(&info->lock); + return ret; } static int noon010_base_config(struct v4l2_subdev *sd) @@ -550,8 +586,6 @@ static int noon010_base_config(struct v4l2_subdev *sd) } if (!ret) ret = noon010_set_flip(sd, 1, 0); - if (!ret) - ret = noon010_power_ctrl(sd, false, false); /* sync the handler and the registers state */ v4l2_ctrl_handler_setup(&to_noon010(sd)->hdl); @@ -582,6 +616,26 @@ static int noon010_s_power(struct v4l2_subdev *sd, int on) return ret; } +static int noon010_s_stream(struct v4l2_subdev *sd, int on) +{ + struct noon010_info *info = to_noon010(sd); + int ret = 0; + + mutex_lock(&info->lock); + if (!info->streaming != !on) { + ret = noon010_power_ctrl(sd, false, !on); + if (!ret) + info->streaming = on; + } + if (!ret && on && info->apply_new_cfg) { + ret = noon010_set_params(sd); + if (!ret) + info->apply_new_cfg = 0; + } + mutex_unlock(&info->lock); + return ret; +} + static int noon010_g_chip_ident(struct v4l2_subdev *sd, struct v4l2_dbg_chip_ident *chip) { @@ -599,6 +653,22 @@ static int noon010_log_status(struct v4l2_subdev *sd) return 0; } +static int noon010_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh) +{ + struct v4l2_mbus_framefmt *mf = v4l2_subdev_get_try_format(fh, 0); + + mf->width = noon010_sizes[0].width; + mf->height = noon010_sizes[0].height; + mf->code = noon010_formats[0].code; + mf->colorspace = V4L2_COLORSPACE_JPEG; + mf->field = V4L2_FIELD_NONE; + return 0; +} + +static const struct v4l2_subdev_internal_ops noon010_subdev_internal_ops = { + .open = noon010_open, +}; + static const struct v4l2_ctrl_ops noon010_ctrl_ops = { .s_ctrl = noon010_s_ctrl, }; @@ -616,15 +686,19 @@ static const struct v4l2_subdev_core_ops noon010_core_ops = { .log_status = noon010_log_status, }; -static const struct v4l2_subdev_video_ops noon010_video_ops = { - .g_mbus_fmt = noon010_g_fmt, - .s_mbus_fmt = noon010_s_fmt, - .try_mbus_fmt = noon010_try_fmt, - .enum_mbus_fmt = noon010_enum_fmt, +static struct v4l2_subdev_pad_ops noon010_pad_ops = { + .enum_mbus_code = noon010_enum_mbus_code, + .get_fmt = noon010_get_fmt, + .set_fmt = noon010_set_fmt, +}; + +static struct v4l2_subdev_video_ops noon010_video_ops = { + .s_stream = noon010_s_stream, }; static const struct v4l2_subdev_ops noon010_ops = { .core = &noon010_core_ops, + .pad = &noon010_pad_ops, .video = &noon010_video_ops, }; @@ -665,10 +739,14 @@ static int noon010_probe(struct i2c_client *client, if (!info) return -ENOMEM; + mutex_init(&info->lock); sd = &info->sd; strlcpy(sd->name, MODULE_NAME, sizeof(sd->name)); v4l2_i2c_subdev_init(sd, client, &noon010_ops); + sd->internal_ops = &noon010_subdev_internal_ops; + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; + v4l2_ctrl_handler_init(&info->hdl, 3); v4l2_ctrl_new_std(&info->hdl, &noon010_ctrl_ops, @@ -719,11 +797,17 @@ static int noon010_probe(struct i2c_client *client, if (ret) goto np_reg_err; + info->pad.flags = MEDIA_PAD_FL_SOURCE; + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR; + ret = media_entity_init(&sd->entity, 1, &info->pad, 0); + if (ret < 0) + goto np_me_err; + ret = noon010_detect(client, info); if (!ret) return 0; - /* the sensor detection failed */ +np_me_err: regulator_bulk_free(NOON010_NUM_SUPPLIES, info->supply); np_reg_err: if (gpio_is_valid(info->gpio_nstby)) @@ -754,6 +838,7 @@ static int noon010_remove(struct i2c_client *client) if (gpio_is_valid(info->gpio_nstby)) gpio_free(info->gpio_nstby); + media_entity_cleanup(&sd->entity); kfree(info); return 0; } -- 1.7.6.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4] noon010pc30: Conversion to the media controller API 2011-09-21 14:26 ` [PATCH v4] " Sylwester Nawrocki @ 2011-09-21 14:28 ` Laurent Pinchart 0 siblings, 0 replies; 9+ messages in thread From: Laurent Pinchart @ 2011-09-21 14:28 UTC (permalink / raw) To: Sylwester Nawrocki; +Cc: linux-media, kyungmin.park, m.szyprowski Hi Sylwester, Thanks for the patch. On Wednesday 21 September 2011 16:26:00 Sylwester Nawrocki wrote: > Replace g/s_mbus_fmt ops with the pad level get/set_fmt operations. > Add media entity initialization and set subdev flags so the host driver > creates a subdev device node for the driver. > A mutex was added for serializing the subdev operations. When setting > format is attempted during streaming an (EBUSY) error will be returned. > > After the device is powered up it will now remain in "power sleep" > mode until s_stream(1) is called. The "power sleep" mode is used > to suspend/resume frame generation at the sensor's output through > s_stream op. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] noon010pc30: Improve s_power operation handling 2011-09-16 15:59 [PATCH v3 0/3] Conversion of the NOON010PC30 sensor driver to media controller API Sylwester Nawrocki 2011-09-16 15:59 ` [PATCH v3 1/3] noon010pc30: Conversion to the " Sylwester Nawrocki @ 2011-09-16 15:59 ` Sylwester Nawrocki 2011-09-16 15:59 ` [PATCH 3/3 (resend)] noon010pc30: Remove g_chip_ident operation handler Sylwester Nawrocki 2 siblings, 0 replies; 9+ messages in thread From: Sylwester Nawrocki @ 2011-09-16 15:59 UTC (permalink / raw) To: linux-media Cc: m.szyprowski, kyungmin.park, laurent.pinchart, s.nawrocki, sw0312.kim, riverful.kim Remove the now unneeded check for the platform data in s_power handler and the platform data pointer in struct noon010_info. Also do not reset the configured output resolution and pixel format when cycling sensor's power. Add small delay for proper reset signal shape. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- drivers/media/video/noon010pc30.c | 40 +++++++++++++++--------------------- 1 files changed, 17 insertions(+), 23 deletions(-) diff --git a/drivers/media/video/noon010pc30.c b/drivers/media/video/noon010pc30.c index 115d976..436b1ee 100644 --- a/drivers/media/video/noon010pc30.c +++ b/drivers/media/video/noon010pc30.c @@ -132,7 +132,6 @@ struct noon010_info { struct v4l2_subdev sd; struct media_pad pad; struct v4l2_ctrl_handler hdl; - const struct noon010pc30_platform_data *pdata; struct regulator_bulk_data supply[NOON010_NUM_SUPPLIES]; u32 gpio_nreset; u32 gpio_nstby; @@ -282,8 +281,10 @@ static int noon010_power_ctrl(struct v4l2_subdev *sd, bool reset, bool sleep) u8 reg = sleep ? 0xF1 : 0xF0; int ret = 0; - if (reset) + if (reset) { ret = cam_i2c_write(sd, POWER_CTRL_REG, reg | 0x02); + udelay(20); + } if (!ret) { ret = cam_i2c_write(sd, POWER_CTRL_REG, reg); if (reset && !ret) @@ -561,45 +562,37 @@ static int noon010_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, return ret; } +/* Called with struct noon010_info.lock mutex held */ static int noon010_base_config(struct v4l2_subdev *sd) { - struct noon010_info *info = to_noon010(sd); - int ret; - - ret = noon010_bulk_write_reg(sd, noon010_base_regs); - if (!ret) { - info->curr_fmt = &noon010_formats[0]; - info->curr_win = &noon010_sizes[0]; + int ret = noon010_bulk_write_reg(sd, noon010_base_regs); + if (!ret) ret = noon010_set_params(sd); - } if (!ret) ret = noon010_set_flip(sd, 1, 0); - /* sync the handler and the registers state */ - v4l2_ctrl_handler_setup(&to_noon010(sd)->hdl); return ret; } static int noon010_s_power(struct v4l2_subdev *sd, int on) { struct noon010_info *info = to_noon010(sd); - const struct noon010pc30_platform_data *pdata = info->pdata; - int ret = 0; - - if (WARN(pdata == NULL, "No platform data!\n")) - return -ENOMEM; + int ret; + mutex_lock(&info->lock); if (on) { ret = power_enable(info); - if (ret) - return ret; - ret = noon010_base_config(sd); + if (!ret) + ret = noon010_base_config(sd); } else { noon010_power_ctrl(sd, false, true); ret = power_disable(info); - info->curr_win = NULL; - info->curr_fmt = NULL; } + mutex_unlock(&info->lock); + + /* Restore the controls state */ + if (!ret && on) + ret = v4l2_ctrl_handler_setup(&info->hdl); return ret; } @@ -750,10 +743,11 @@ static int noon010_probe(struct i2c_client *client, if (ret) goto np_err; - info->pdata = client->dev.platform_data; info->i2c_reg_page = -1; info->gpio_nreset = -EINVAL; info->gpio_nstby = -EINVAL; + info->curr_fmt = &noon010_formats[0]; + info->curr_win = &noon010_sizes[0]; if (gpio_is_valid(pdata->gpio_nreset)) { ret = gpio_request(pdata->gpio_nreset, "NOON010PC30 NRST"); -- 1.7.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3 (resend)] noon010pc30: Remove g_chip_ident operation handler 2011-09-16 15:59 [PATCH v3 0/3] Conversion of the NOON010PC30 sensor driver to media controller API Sylwester Nawrocki 2011-09-16 15:59 ` [PATCH v3 1/3] noon010pc30: Conversion to the " Sylwester Nawrocki 2011-09-16 15:59 ` [PATCH v3 2/3] noon010pc30: Improve s_power operation handling Sylwester Nawrocki @ 2011-09-16 15:59 ` Sylwester Nawrocki 2 siblings, 0 replies; 9+ messages in thread From: Sylwester Nawrocki @ 2011-09-16 15:59 UTC (permalink / raw) To: linux-media Cc: m.szyprowski, kyungmin.park, laurent.pinchart, s.nawrocki, sw0312.kim, riverful.kim It is now not needed as the sensor identification is done through the media controller API. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> --- drivers/media/video/noon010pc30.c | 10 ---------- include/media/v4l2-chip-ident.h | 3 --- 2 files changed, 0 insertions(+), 13 deletions(-) diff --git a/drivers/media/video/noon010pc30.c b/drivers/media/video/noon010pc30.c index 436b1ee..d38b4d4 100644 --- a/drivers/media/video/noon010pc30.c +++ b/drivers/media/video/noon010pc30.c @@ -617,15 +617,6 @@ static int noon010_s_stream(struct v4l2_subdev *sd, int on) return ret; } -static int noon010_g_chip_ident(struct v4l2_subdev *sd, - struct v4l2_dbg_chip_ident *chip) -{ - struct i2c_client *client = v4l2_get_subdevdata(sd); - - return v4l2_chip_ident_i2c_client(client, chip, - V4L2_IDENT_NOON010PC30, 0); -} - static int noon010_log_status(struct v4l2_subdev *sd) { struct noon010_info *info = to_noon010(sd); @@ -655,7 +646,6 @@ static const struct v4l2_ctrl_ops noon010_ctrl_ops = { }; static const struct v4l2_subdev_core_ops noon010_core_ops = { - .g_chip_ident = noon010_g_chip_ident, .s_power = noon010_s_power, .g_ctrl = v4l2_subdev_g_ctrl, .s_ctrl = v4l2_subdev_s_ctrl, diff --git a/include/media/v4l2-chip-ident.h b/include/media/v4l2-chip-ident.h index 63fd9d3..810a209 100644 --- a/include/media/v4l2-chip-ident.h +++ b/include/media/v4l2-chip-ident.h @@ -212,9 +212,6 @@ enum { /* module sn9c20x: just ident 10000 */ V4L2_IDENT_SN9C20X = 10000, - /* Siliconfile sensors: reserved range 10100 - 10199 */ - V4L2_IDENT_NOON010PC30 = 10100, - /* module cx231xx and cx25840 */ V4L2_IDENT_CX2310X_AV = 23099, /* Integrated A/V decoder; not in '100 */ V4L2_IDENT_CX23100 = 23100, -- 1.7.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-09-21 14:28 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-16 15:59 [PATCH v3 0/3] Conversion of the NOON010PC30 sensor driver to media controller API Sylwester Nawrocki 2011-09-16 15:59 ` [PATCH v3 1/3] noon010pc30: Conversion to the " Sylwester Nawrocki 2011-09-16 16:17 ` Sylwester Nawrocki 2011-09-20 22:18 ` Laurent Pinchart 2011-09-21 13:54 ` Sylwester Nawrocki 2011-09-21 14:26 ` [PATCH v4] " Sylwester Nawrocki 2011-09-21 14:28 ` Laurent Pinchart 2011-09-16 15:59 ` [PATCH v3 2/3] noon010pc30: Improve s_power operation handling Sylwester Nawrocki 2011-09-16 15:59 ` [PATCH 3/3 (resend)] noon010pc30: Remove g_chip_ident operation handler Sylwester Nawrocki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.