| Message ID | 20240430103956.60190-11-jacopo.mondi@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series | [01/19] media: adv748x: Add support for active state | expand |
Hi Jacopo, Thank you for the patch. On Tue, Apr 30, 2024 at 12:39:46PM +0200, Jacopo Mondi wrote: > Use the frame interval stored in the subdev state instead of storing > a copy in the driver private structure. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > drivers/media/i2c/max9286.c | 52 ++++++++----------------------------- > 1 file changed, 11 insertions(+), 41 deletions(-) > > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c > index 575d8ac8efa1..ae1b73fde832 100644 > --- a/drivers/media/i2c/max9286.c > +++ b/drivers/media/i2c/max9286.c > @@ -197,8 +197,6 @@ struct max9286_priv { > struct v4l2_ctrl *pixelrate_ctrl; > unsigned int pixelrate; > > - struct v4l2_fract interval; > - > unsigned int nsources; > unsigned int source_mask; > unsigned int route_mask; > @@ -571,11 +569,14 @@ static void max9286_set_video_format(struct max9286_priv *priv, > MAX9286_INVVS | MAX9286_HVSRC_D14); > } > > -static void max9286_set_fsync_period(struct max9286_priv *priv) > +static void max9286_set_fsync_period(struct max9286_priv *priv, > + struct v4l2_subdev_state *state) > { > + struct v4l2_fract *interval; > u32 fsync; > > - if (!priv->interval.numerator || !priv->interval.denominator) { > + interval = v4l2_subdev_state_get_interval(state, MAX9286_SRC_PAD); > + if (!interval->numerator || !interval->denominator) { > /* > * Special case, a null interval enables automatic FRAMESYNC > * mode. FRAMESYNC is taken from the slowest link. > @@ -591,8 +592,8 @@ static void max9286_set_fsync_period(struct max9286_priv *priv) > * The FRAMESYNC generator is configured with a period expressed as a > * number of PCLK periods. > */ > - fsync = div_u64((u64)priv->pixelrate * priv->interval.numerator, > - priv->interval.denominator); > + fsync = div_u64((u64)priv->pixelrate * interval->numerator, > + interval->denominator); > > dev_dbg(&priv->client->dev, "fsync period %u (pclk %u)\n", fsync, > priv->pixelrate); > @@ -802,7 +803,7 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) > format = v4l2_subdev_state_get_format(state, source_idx); > > max9286_set_video_format(priv, format); > - max9286_set_fsync_period(priv); > + max9286_set_fsync_period(priv, state); > > /* > * The frame sync between cameras is transmitted across the > @@ -871,44 +872,14 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) > return ret; > } > > -static int max9286_get_frame_interval(struct v4l2_subdev *sd, > - struct v4l2_subdev_state *sd_state, > - struct v4l2_subdev_frame_interval *interval) > -{ > - struct max9286_priv *priv = sd_to_max9286(sd); > - > - /* > - * FIXME: Implement support for V4L2_SUBDEV_FORMAT_TRY, using the V4L2 > - * subdev active state API. > - */ > - if (interval->which != V4L2_SUBDEV_FORMAT_ACTIVE) > - return -EINVAL; > - > - if (interval->pad != MAX9286_SRC_PAD) > - return -EINVAL; You will now return a value for the sink pads. Is that fine ? > - > - interval->interval = priv->interval; > - > - return 0; > -} > - > static int max9286_set_frame_interval(struct v4l2_subdev *sd, > - struct v4l2_subdev_state *sd_state, > + struct v4l2_subdev_state *state, > struct v4l2_subdev_frame_interval *interval) > { > - struct max9286_priv *priv = sd_to_max9286(sd); > - > - /* > - * FIXME: Implement support for V4L2_SUBDEV_FORMAT_TRY, using the V4L2 > - * subdev active state API. > - */ > - if (interval->which != V4L2_SUBDEV_FORMAT_ACTIVE) > - return -EINVAL; > - > if (interval->pad != MAX9286_SRC_PAD) > return -EINVAL; > > - priv->interval = interval->interval; > + *v4l2_subdev_state_get_interval(state, interval->pad) = interval->interval; > > return 0; > } > @@ -966,7 +937,7 @@ static const struct v4l2_subdev_pad_ops max9286_pad_ops = { > .enum_mbus_code = max9286_enum_mbus_code, > .get_fmt = v4l2_subdev_get_fmt, > .set_fmt = max9286_set_fmt, > - .get_frame_interval = max9286_get_frame_interval, > + .get_frame_interval = v4l2_subdev_get_frame_interval, In otder to do so, you need to initialize the interval in the .init_state() operation. > .set_frame_interval = max9286_set_frame_interval, > }; > > @@ -1148,7 +1119,6 @@ static int max9286_setup(struct max9286_priv *priv) > max9286_write(priv, 0x69, (0xf & ~priv->route_mask)); > > max9286_set_video_format(priv, &max9286_default_format); > - max9286_set_fsync_period(priv); > > cfg = max9286_read(priv, 0x1c); > if (cfg < 0)
diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c index 575d8ac8efa1..ae1b73fde832 100644 --- a/drivers/media/i2c/max9286.c +++ b/drivers/media/i2c/max9286.c @@ -197,8 +197,6 @@ struct max9286_priv { struct v4l2_ctrl *pixelrate_ctrl; unsigned int pixelrate; - struct v4l2_fract interval; - unsigned int nsources; unsigned int source_mask; unsigned int route_mask; @@ -571,11 +569,14 @@ static void max9286_set_video_format(struct max9286_priv *priv, MAX9286_INVVS | MAX9286_HVSRC_D14); } -static void max9286_set_fsync_period(struct max9286_priv *priv) +static void max9286_set_fsync_period(struct max9286_priv *priv, + struct v4l2_subdev_state *state) { + struct v4l2_fract *interval; u32 fsync; - if (!priv->interval.numerator || !priv->interval.denominator) { + interval = v4l2_subdev_state_get_interval(state, MAX9286_SRC_PAD); + if (!interval->numerator || !interval->denominator) { /* * Special case, a null interval enables automatic FRAMESYNC * mode. FRAMESYNC is taken from the slowest link. @@ -591,8 +592,8 @@ static void max9286_set_fsync_period(struct max9286_priv *priv) * The FRAMESYNC generator is configured with a period expressed as a * number of PCLK periods. */ - fsync = div_u64((u64)priv->pixelrate * priv->interval.numerator, - priv->interval.denominator); + fsync = div_u64((u64)priv->pixelrate * interval->numerator, + interval->denominator); dev_dbg(&priv->client->dev, "fsync period %u (pclk %u)\n", fsync, priv->pixelrate); @@ -802,7 +803,7 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) format = v4l2_subdev_state_get_format(state, source_idx); max9286_set_video_format(priv, format); - max9286_set_fsync_period(priv); + max9286_set_fsync_period(priv, state); /* * The frame sync between cameras is transmitted across the @@ -871,44 +872,14 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable) return ret; } -static int max9286_get_frame_interval(struct v4l2_subdev *sd, - struct v4l2_subdev_state *sd_state, - struct v4l2_subdev_frame_interval *interval) -{ - struct max9286_priv *priv = sd_to_max9286(sd); - - /* - * FIXME: Implement support for V4L2_SUBDEV_FORMAT_TRY, using the V4L2 - * subdev active state API. - */ - if (interval->which != V4L2_SUBDEV_FORMAT_ACTIVE) - return -EINVAL; - - if (interval->pad != MAX9286_SRC_PAD) - return -EINVAL; - - interval->interval = priv->interval; - - return 0; -} - static int max9286_set_frame_interval(struct v4l2_subdev *sd, - struct v4l2_subdev_state *sd_state, + struct v4l2_subdev_state *state, struct v4l2_subdev_frame_interval *interval) { - struct max9286_priv *priv = sd_to_max9286(sd); - - /* - * FIXME: Implement support for V4L2_SUBDEV_FORMAT_TRY, using the V4L2 - * subdev active state API. - */ - if (interval->which != V4L2_SUBDEV_FORMAT_ACTIVE) - return -EINVAL; - if (interval->pad != MAX9286_SRC_PAD) return -EINVAL; - priv->interval = interval->interval; + *v4l2_subdev_state_get_interval(state, interval->pad) = interval->interval; return 0; } @@ -966,7 +937,7 @@ static const struct v4l2_subdev_pad_ops max9286_pad_ops = { .enum_mbus_code = max9286_enum_mbus_code, .get_fmt = v4l2_subdev_get_fmt, .set_fmt = max9286_set_fmt, - .get_frame_interval = max9286_get_frame_interval, + .get_frame_interval = v4l2_subdev_get_frame_interval, .set_frame_interval = max9286_set_frame_interval, }; @@ -1148,7 +1119,6 @@ static int max9286_setup(struct max9286_priv *priv) max9286_write(priv, 0x69, (0xf & ~priv->route_mask)); max9286_set_video_format(priv, &max9286_default_format); - max9286_set_fsync_period(priv); cfg = max9286_read(priv, 0x1c); if (cfg < 0)
Use the frame interval stored in the subdev state instead of storing a copy in the driver private structure. Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- drivers/media/i2c/max9286.c | 52 ++++++++----------------------------- 1 file changed, 11 insertions(+), 41 deletions(-)