| Message ID | 20230821223001.28480-12-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: imx219: Miscellaneous cleanups and improvements | expand |
Hi Laurent On Tue, Aug 22, 2023 at 01:29:54AM +0300, Laurent Pinchart wrote: > The mode field of the imx219 structure is only used in > imx219_init_controls(), after the probe function sets it to point to the > default mode. Use the default mode directly when initializing controls, > and drop the mode field from the imx219 structure. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/i2c/imx219.c | 20 +++++++------------- > 1 file changed, 7 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index 4e9a50117a0a..67a30dc39641 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -363,9 +363,6 @@ struct imx219 { > struct v4l2_ctrl *vblank; > struct v4l2_ctrl *hblank; > > - /* Current mode */ > - const struct imx219_mode *mode; > - > /* Streaming on/off */ > bool streaming; > > @@ -584,7 +581,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd, > *crop = mode->crop; > > if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) { > - imx219->mode = mode; > /* Update limits and set FPS to default */ > __v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN, > IMX219_VTS_MAX - mode->height, 1, > @@ -967,8 +963,8 @@ static unsigned long imx219_get_pixel_rate(struct imx219 *imx219) > static int imx219_init_controls(struct imx219 *imx219) > { > struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + const struct imx219_mode *mode = &supported_modes[0]; > struct v4l2_ctrl_handler *ctrl_hdlr; > - unsigned int height = imx219->mode->height; > struct v4l2_fwnode_device_properties props; > int exposure_max, exposure_def, hblank; > int i, ret; > @@ -997,15 +993,15 @@ static int imx219_init_controls(struct imx219 *imx219) > /* Initial vblank/hblank/exposure parameters based on current mode */ > imx219->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > V4L2_CID_VBLANK, IMX219_VBLANK_MIN, > - IMX219_VTS_MAX - height, 1, > - imx219->mode->vts_def - height); > - hblank = IMX219_PPL_DEFAULT - imx219->mode->width; > + IMX219_VTS_MAX - mode->height, 1, > + mode->vts_def - mode->height); > + hblank = IMX219_PPL_DEFAULT - mode->width; > imx219->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > V4L2_CID_HBLANK, hblank, hblank, > 1, hblank); > if (imx219->hblank) > imx219->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; > - exposure_max = imx219->mode->vts_def - 4; > + exposure_max = mode->vts_def - 4; > exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ? > exposure_max : IMX219_EXPOSURE_DEFAULT; > imx219->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > @@ -1192,10 +1188,8 @@ static int imx219_probe(struct i2c_client *client) > if (ret) > goto error_power_off; > > - /* Set default mode to max resolution */ > - imx219->mode = &supported_modes[0]; > - > - /* sensor doesn't enter LP-11 state upon power up until and unless > + /* > + * Sensor doesn't enter LP-11 state upon power up until and unless Unrelated but doesn't hurt Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > * streaming is started, so upon power up switch the modes to: > * streaming -> standby > */ > -- > Regards, > > Laurent Pinchart >
On Mon, 21 Aug 2023 at 23:30, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > The mode field of the imx219 structure is only used in > imx219_init_controls(), after the probe function sets it to point to the > default mode. Use the default mode directly when initializing controls, > and drop the mode field from the imx219 structure. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > drivers/media/i2c/imx219.c | 20 +++++++------------- > 1 file changed, 7 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index 4e9a50117a0a..67a30dc39641 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -363,9 +363,6 @@ struct imx219 { > struct v4l2_ctrl *vblank; > struct v4l2_ctrl *hblank; > > - /* Current mode */ > - const struct imx219_mode *mode; > - > /* Streaming on/off */ > bool streaming; > > @@ -584,7 +581,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd, > *crop = mode->crop; > > if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) { > - imx219->mode = mode; > /* Update limits and set FPS to default */ > __v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN, > IMX219_VTS_MAX - mode->height, 1, > @@ -967,8 +963,8 @@ static unsigned long imx219_get_pixel_rate(struct imx219 *imx219) > static int imx219_init_controls(struct imx219 *imx219) > { > struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); > + const struct imx219_mode *mode = &supported_modes[0]; > struct v4l2_ctrl_handler *ctrl_hdlr; > - unsigned int height = imx219->mode->height; > struct v4l2_fwnode_device_properties props; > int exposure_max, exposure_def, hblank; > int i, ret; > @@ -997,15 +993,15 @@ static int imx219_init_controls(struct imx219 *imx219) > /* Initial vblank/hblank/exposure parameters based on current mode */ > imx219->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > V4L2_CID_VBLANK, IMX219_VBLANK_MIN, > - IMX219_VTS_MAX - height, 1, > - imx219->mode->vts_def - height); > - hblank = IMX219_PPL_DEFAULT - imx219->mode->width; > + IMX219_VTS_MAX - mode->height, 1, > + mode->vts_def - mode->height); > + hblank = IMX219_PPL_DEFAULT - mode->width; > imx219->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > V4L2_CID_HBLANK, hblank, hblank, > 1, hblank); > if (imx219->hblank) > imx219->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; > - exposure_max = imx219->mode->vts_def - 4; > + exposure_max = mode->vts_def - 4; > exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ? > exposure_max : IMX219_EXPOSURE_DEFAULT; > imx219->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, > @@ -1192,10 +1188,8 @@ static int imx219_probe(struct i2c_client *client) > if (ret) > goto error_power_off; > > - /* Set default mode to max resolution */ > - imx219->mode = &supported_modes[0]; > - > - /* sensor doesn't enter LP-11 state upon power up until and unless > + /* > + * Sensor doesn't enter LP-11 state upon power up until and unless > * streaming is started, so upon power up switch the modes to: > * streaming -> standby > */ > -- > Regards, > > Laurent Pinchart >
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index 4e9a50117a0a..67a30dc39641 100644 --- a/drivers/media/i2c/imx219.c +++ b/drivers/media/i2c/imx219.c @@ -363,9 +363,6 @@ struct imx219 { struct v4l2_ctrl *vblank; struct v4l2_ctrl *hblank; - /* Current mode */ - const struct imx219_mode *mode; - /* Streaming on/off */ bool streaming; @@ -584,7 +581,6 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd, *crop = mode->crop; if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) { - imx219->mode = mode; /* Update limits and set FPS to default */ __v4l2_ctrl_modify_range(imx219->vblank, IMX219_VBLANK_MIN, IMX219_VTS_MAX - mode->height, 1, @@ -967,8 +963,8 @@ static unsigned long imx219_get_pixel_rate(struct imx219 *imx219) static int imx219_init_controls(struct imx219 *imx219) { struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd); + const struct imx219_mode *mode = &supported_modes[0]; struct v4l2_ctrl_handler *ctrl_hdlr; - unsigned int height = imx219->mode->height; struct v4l2_fwnode_device_properties props; int exposure_max, exposure_def, hblank; int i, ret; @@ -997,15 +993,15 @@ static int imx219_init_controls(struct imx219 *imx219) /* Initial vblank/hblank/exposure parameters based on current mode */ imx219->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, V4L2_CID_VBLANK, IMX219_VBLANK_MIN, - IMX219_VTS_MAX - height, 1, - imx219->mode->vts_def - height); - hblank = IMX219_PPL_DEFAULT - imx219->mode->width; + IMX219_VTS_MAX - mode->height, 1, + mode->vts_def - mode->height); + hblank = IMX219_PPL_DEFAULT - mode->width; imx219->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, V4L2_CID_HBLANK, hblank, hblank, 1, hblank); if (imx219->hblank) imx219->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY; - exposure_max = imx219->mode->vts_def - 4; + exposure_max = mode->vts_def - 4; exposure_def = (exposure_max < IMX219_EXPOSURE_DEFAULT) ? exposure_max : IMX219_EXPOSURE_DEFAULT; imx219->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops, @@ -1192,10 +1188,8 @@ static int imx219_probe(struct i2c_client *client) if (ret) goto error_power_off; - /* Set default mode to max resolution */ - imx219->mode = &supported_modes[0]; - - /* sensor doesn't enter LP-11 state upon power up until and unless + /* + * Sensor doesn't enter LP-11 state upon power up until and unless * streaming is started, so upon power up switch the modes to: * streaming -> standby */
The mode field of the imx219 structure is only used in imx219_init_controls(), after the probe function sets it to point to the default mode. Use the default mode directly when initializing controls, and drop the mode field from the imx219 structure. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/i2c/imx219.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)