| Message ID | 20250219-imx219_fixes_v2-v1-3-0e3f5dd9b024@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: i2c: imx219: Follow-up on binning fixes | expand |
On Wed, Feb 19, 2025 at 05:16:45PM +0530, Jai Luthra wrote: > The pixel rate and binning calculations need the format and resolution > of the sensor, so pass the v4l2 subdev state directly instead of always > operating on the active state. Yeah I think it's saner, as even if v4l2 ctrl do not support TRY state (and thus the pixel rate calculation can't be "tryed") we should use the format information from the state at hand. > > Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > Link: https://lore.kernel.org/linux-media/sejl7xskif6rlpdsg3jhczjwe5gi6rs53ehbyka6omv2zeg7qq@4iis7i2lla5p/ > Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > --- > drivers/media/i2c/imx219.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c > index f02732d8fa95de0a295f247d4f0b60017dbb2ed2..0adfe8e5775ba6661f7d06fedfd920d91c24cba5 100644 > --- a/drivers/media/i2c/imx219.c > +++ b/drivers/media/i2c/imx219.c > @@ -400,10 +400,9 @@ static u32 imx219_get_format_bpp(const struct v4l2_mbus_framefmt *format) > } > } > > -static void imx219_get_binning(struct imx219 *imx219, u8 *bin_h, u8 *bin_v) > +static void imx219_get_binning(struct v4l2_subdev_state *state, u8 *bin_h, > + u8 *bin_v) > { > - struct v4l2_subdev_state *state = > - v4l2_subdev_get_locked_active_state(&imx219->sd); > const struct v4l2_mbus_framefmt *format = > v4l2_subdev_state_get_format(state, 0); > const struct v4l2_rect *crop = v4l2_subdev_state_get_crop(state, 0); > @@ -430,11 +429,11 @@ static void imx219_get_binning(struct imx219 *imx219, u8 *bin_h, u8 *bin_v) > *bin_v = IMX219_BINNING_X2; > } > > -static inline u32 imx219_get_rate_factor(struct imx219 *imx219) > +static inline u32 imx219_get_rate_factor(struct v4l2_subdev_state *state) > { > u8 bin_h, bin_v; > > - imx219_get_binning(imx219, &bin_h, &bin_v); > + imx219_get_binning(state, &bin_h, &bin_v); > > return (bin_h & bin_v) == IMX219_BINNING_X2_ANALOG ? 2 : 1; > } > @@ -455,7 +454,7 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl) > > state = v4l2_subdev_get_locked_active_state(&imx219->sd); > format = v4l2_subdev_state_get_format(state, 0); > - rate_factor = imx219_get_rate_factor(imx219); > + rate_factor = imx219_get_rate_factor(state); > > if (ctrl->id == V4L2_CID_VBLANK) { > int exposure_max, exposure_def; > @@ -689,7 +688,7 @@ static int imx219_set_framefmt(struct imx219 *imx219, > cci_write(imx219->regmap, IMX219_REG_Y_ADD_END_A, > crop->top - IMX219_PIXEL_ARRAY_TOP + crop->height - 1, &ret); > > - imx219_get_binning(imx219, &bin_h, &bin_v); > + imx219_get_binning(state, &bin_h, &bin_v); > cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_H, bin_h, &ret); > cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_V, bin_v, &ret); > > @@ -937,7 +936,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd, > > /* Scale the pixel rate based on the mode specific factor */ > pixel_rate = imx219_get_pixel_rate(imx219) * > - imx219_get_rate_factor(imx219); > + imx219_get_rate_factor(state); > __v4l2_ctrl_modify_range(imx219->pixel_rate, pixel_rate, > pixel_rate, 1, pixel_rate); > } > > -- > 2.48.1 >
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c index f02732d8fa95de0a295f247d4f0b60017dbb2ed2..0adfe8e5775ba6661f7d06fedfd920d91c24cba5 100644 --- a/drivers/media/i2c/imx219.c +++ b/drivers/media/i2c/imx219.c @@ -400,10 +400,9 @@ static u32 imx219_get_format_bpp(const struct v4l2_mbus_framefmt *format) } } -static void imx219_get_binning(struct imx219 *imx219, u8 *bin_h, u8 *bin_v) +static void imx219_get_binning(struct v4l2_subdev_state *state, u8 *bin_h, + u8 *bin_v) { - struct v4l2_subdev_state *state = - v4l2_subdev_get_locked_active_state(&imx219->sd); const struct v4l2_mbus_framefmt *format = v4l2_subdev_state_get_format(state, 0); const struct v4l2_rect *crop = v4l2_subdev_state_get_crop(state, 0); @@ -430,11 +429,11 @@ static void imx219_get_binning(struct imx219 *imx219, u8 *bin_h, u8 *bin_v) *bin_v = IMX219_BINNING_X2; } -static inline u32 imx219_get_rate_factor(struct imx219 *imx219) +static inline u32 imx219_get_rate_factor(struct v4l2_subdev_state *state) { u8 bin_h, bin_v; - imx219_get_binning(imx219, &bin_h, &bin_v); + imx219_get_binning(state, &bin_h, &bin_v); return (bin_h & bin_v) == IMX219_BINNING_X2_ANALOG ? 2 : 1; } @@ -455,7 +454,7 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl) state = v4l2_subdev_get_locked_active_state(&imx219->sd); format = v4l2_subdev_state_get_format(state, 0); - rate_factor = imx219_get_rate_factor(imx219); + rate_factor = imx219_get_rate_factor(state); if (ctrl->id == V4L2_CID_VBLANK) { int exposure_max, exposure_def; @@ -689,7 +688,7 @@ static int imx219_set_framefmt(struct imx219 *imx219, cci_write(imx219->regmap, IMX219_REG_Y_ADD_END_A, crop->top - IMX219_PIXEL_ARRAY_TOP + crop->height - 1, &ret); - imx219_get_binning(imx219, &bin_h, &bin_v); + imx219_get_binning(state, &bin_h, &bin_v); cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_H, bin_h, &ret); cci_write(imx219->regmap, IMX219_REG_BINNING_MODE_V, bin_v, &ret); @@ -937,7 +936,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd, /* Scale the pixel rate based on the mode specific factor */ pixel_rate = imx219_get_pixel_rate(imx219) * - imx219_get_rate_factor(imx219); + imx219_get_rate_factor(state); __v4l2_ctrl_modify_range(imx219->pixel_rate, pixel_rate, pixel_rate, 1, pixel_rate); }
The pixel rate and binning calculations need the format and resolution of the sensor, so pass the v4l2 subdev state directly instead of always operating on the active state. Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Link: https://lore.kernel.org/linux-media/sejl7xskif6rlpdsg3jhczjwe5gi6rs53ehbyka6omv2zeg7qq@4iis7i2lla5p/ Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com> --- drivers/media/i2c/imx219.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)