diff mbox series

[v2,11/11] media: max9286: Use frame interval from subdev state

Message ID 20240506164941.110389-12-jacopo.mondi@ideasonboard.com (mailing list archive)
State New
Headers
Series media: renesas: rcar-csi2: Use the subdev active state |

Commit Message

Jacopo Mondi May 6, 2024, 4:49 p.m. UTC 
  Use the frame interval stored in the subdev state instead of storing
a copy in the driver private structure.

Initialize the frame interval to the special case 0/0 that in the
max9286 driver represents automatic handling of frame sync.

During the startup phase, configure register 0x01 to use automatic
frame sync, to match the subdev state initialiation, instead of calling
max9286_set_fsync_period() which now requires a 'state' argument.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 drivers/media/i2c/max9286.c | 59 +++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 28 deletions(-)

Comments

Laurent Pinchart May 9, 2024, 12:51 p.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Mon, May 06, 2024 at 06:49:39PM +0200, Jacopo Mondi wrote:
> Use the frame interval stored in the subdev state instead of storing
> a copy in the driver private structure.
> 
> Initialize the frame interval to the special case 0/0 that in the
> max9286 driver represents automatic handling of frame sync.
> 
> During the startup phase, configure register 0x01 to use automatic
> frame sync, to match the subdev state initialiation, instead of calling
> max9286_set_fsync_period() which now requires a 'state' argument.

Given that max9286_set_fsync_period() will be called at stream start
time, is there a need to set the register in max9286_setup() ? If so,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

If not, you can drop it.

> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  drivers/media/i2c/max9286.c | 59 +++++++++++++++++++------------------
>  1 file changed, 31 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index 7fad190cd9b3..6930a98c8965 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)

Depending on whether this series or "media: v4l2-subdev: Provide
const-aware subdev state accessors" gets merged first, this argument
could be const :-) No need to address that for now and add a dependency.

>  {
> +	const 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);
> @@ -801,7 +802,7 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable)
>  		format = v4l2_subdev_state_get_format(state, MAX9286_SRC_PAD);
>  
>  		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
> @@ -874,19 +875,11 @@ 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;
> +	interval->interval = *v4l2_subdev_state_get_interval(sd_state,
> +							     interval->pad);
>  
>  	return 0;
>  }
> @@ -895,19 +888,11 @@ static int max9286_set_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;
>  
> -	priv->interval = interval->interval;
> +	*v4l2_subdev_state_get_interval(sd_state,
> +					interval->pad) = interval->interval;
>  
>  	return 0;
>  }
> @@ -993,9 +978,21 @@ static const struct v4l2_mbus_framefmt max9286_default_format = {
>  static int max9286_init_state(struct v4l2_subdev *sd,
>  			      struct v4l2_subdev_state *state)
>  {
> +	struct v4l2_fract *interval;
> +
>  	for (unsigned int i = 0; i < MAX9286_N_PADS; i++)
>  		*v4l2_subdev_state_get_format(state, i) = max9286_default_format;
>  
> +	/*
> +	 * Special case: a null interval enables automatic FRAMESYNC mode.
> +	 *
> +	 * FRAMESYNC is taken from the slowest link. See register 0x01
> +	 * configuration.
> +	 */
> +	interval = v4l2_subdev_state_get_interval(state, MAX9286_SRC_PAD);
> +	interval->numerator = 0;
> +	interval->denominator = 0;
> +
>  	return 0;
>  }
>  
> @@ -1142,7 +1139,13 @@ 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);
> +
> +	/*
> +	 * Use automatic FRAMESYNC mode. FRAMESYNC is taken from the slowest
> +	 * link.
> +	 */
> +	max9286_write(priv, 0x01, MAX9286_FSYNCMODE_INT_HIZ |
> +				  MAX9286_FSYNCMETH_AUTO);
>  
>  	cfg = max9286_read(priv, 0x1c);
>  	if (cfg < 0)
Jacopo Mondi May 9, 2024, 1:45 p.m. UTC | #2 
Hi Laurent

On Thu, May 09, 2024 at 03:51:22PM GMT, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Mon, May 06, 2024 at 06:49:39PM +0200, Jacopo Mondi wrote:
> > Use the frame interval stored in the subdev state instead of storing
> > a copy in the driver private structure.
> >
> > Initialize the frame interval to the special case 0/0 that in the
> > max9286 driver represents automatic handling of frame sync.
> >
> > During the startup phase, configure register 0x01 to use automatic
> > frame sync, to match the subdev state initialiation, instead of calling
> > max9286_set_fsync_period() which now requires a 'state' argument.
>
> Given that max9286_set_fsync_period() will be called at stream start
> time, is there a need to set the register in max9286_setup() ? If so,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> If not, you can drop it.
>

Ok, I removed it initially then re-introduced in v2

> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  drivers/media/i2c/max9286.c | 59 +++++++++++++++++++------------------
> >  1 file changed, 31 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > index 7fad190cd9b3..6930a98c8965 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)
>
> Depending on whether this series or "media: v4l2-subdev: Provide
> const-aware subdev state accessors" gets merged first, this argument
> could be const :-) No need to address that for now and add a dependency.
>
> >  {
> > +	const 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);
> > @@ -801,7 +802,7 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable)
> >  		format = v4l2_subdev_state_get_format(state, MAX9286_SRC_PAD);
> >
> >  		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
> > @@ -874,19 +875,11 @@ 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;
> > +	interval->interval = *v4l2_subdev_state_get_interval(sd_state,
> > +							     interval->pad);
> >
> >  	return 0;
> >  }
> > @@ -895,19 +888,11 @@ static int max9286_set_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;
> >
> > -	priv->interval = interval->interval;
> > +	*v4l2_subdev_state_get_interval(sd_state,
> > +					interval->pad) = interval->interval;
> >
> >  	return 0;
> >  }
> > @@ -993,9 +978,21 @@ static const struct v4l2_mbus_framefmt max9286_default_format = {
> >  static int max9286_init_state(struct v4l2_subdev *sd,
> >  			      struct v4l2_subdev_state *state)
> >  {
> > +	struct v4l2_fract *interval;
> > +
> >  	for (unsigned int i = 0; i < MAX9286_N_PADS; i++)
> >  		*v4l2_subdev_state_get_format(state, i) = max9286_default_format;
> >
> > +	/*
> > +	 * Special case: a null interval enables automatic FRAMESYNC mode.
> > +	 *
> > +	 * FRAMESYNC is taken from the slowest link. See register 0x01
> > +	 * configuration.
> > +	 */
> > +	interval = v4l2_subdev_state_get_interval(state, MAX9286_SRC_PAD);
> > +	interval->numerator = 0;
> > +	interval->denominator = 0;
> > +
> >  	return 0;
> >  }
> >
> > @@ -1142,7 +1139,13 @@ 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);
> > +
> > +	/*
> > +	 * Use automatic FRAMESYNC mode. FRAMESYNC is taken from the slowest
> > +	 * link.
> > +	 */
> > +	max9286_write(priv, 0x01, MAX9286_FSYNCMODE_INT_HIZ |
> > +				  MAX9286_FSYNCMETH_AUTO);
> >
> >  	cfg = max9286_read(priv, 0x1c);
> >  	if (cfg < 0)
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart May 9, 2024, 2:11 p.m. UTC | #3 
Hi Jacopo,

On Thu, May 09, 2024 at 03:45:11PM +0200, Jacopo Mondi wrote:
> On Thu, May 09, 2024 at 03:51:22PM GMT, Laurent Pinchart wrote:
> > On Mon, May 06, 2024 at 06:49:39PM +0200, Jacopo Mondi wrote:
> > > Use the frame interval stored in the subdev state instead of storing
> > > a copy in the driver private structure.
> > >
> > > Initialize the frame interval to the special case 0/0 that in the
> > > max9286 driver represents automatic handling of frame sync.
> > >
> > > During the startup phase, configure register 0x01 to use automatic
> > > frame sync, to match the subdev state initialiation, instead of calling
> > > max9286_set_fsync_period() which now requires a 'state' argument.
> >
> > Given that max9286_set_fsync_period() will be called at stream start
> > time, is there a need to set the register in max9286_setup() ? If so,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > If not, you can drop it.
> 
> Ok, I removed it initially then re-introduced in v2

If you drop it, please just record the rationale in the commit message.

> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > ---
> > >  drivers/media/i2c/max9286.c | 59 +++++++++++++++++++------------------
> > >  1 file changed, 31 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > > index 7fad190cd9b3..6930a98c8965 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)
> >
> > Depending on whether this series or "media: v4l2-subdev: Provide
> > const-aware subdev state accessors" gets merged first, this argument
> > could be const :-) No need to address that for now and add a dependency.
> >
> > >  {
> > > +	const 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);
> > > @@ -801,7 +802,7 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable)
> > >  		format = v4l2_subdev_state_get_format(state, MAX9286_SRC_PAD);
> > >
> > >  		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
> > > @@ -874,19 +875,11 @@ 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;
> > > +	interval->interval = *v4l2_subdev_state_get_interval(sd_state,
> > > +							     interval->pad);
> > >
> > >  	return 0;
> > >  }
> > > @@ -895,19 +888,11 @@ static int max9286_set_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;
> > >
> > > -	priv->interval = interval->interval;
> > > +	*v4l2_subdev_state_get_interval(sd_state,
> > > +					interval->pad) = interval->interval;
> > >
> > >  	return 0;
> > >  }
> > > @@ -993,9 +978,21 @@ static const struct v4l2_mbus_framefmt max9286_default_format = {
> > >  static int max9286_init_state(struct v4l2_subdev *sd,
> > >  			      struct v4l2_subdev_state *state)
> > >  {
> > > +	struct v4l2_fract *interval;
> > > +
> > >  	for (unsigned int i = 0; i < MAX9286_N_PADS; i++)
> > >  		*v4l2_subdev_state_get_format(state, i) = max9286_default_format;
> > >
> > > +	/*
> > > +	 * Special case: a null interval enables automatic FRAMESYNC mode.
> > > +	 *
> > > +	 * FRAMESYNC is taken from the slowest link. See register 0x01
> > > +	 * configuration.
> > > +	 */
> > > +	interval = v4l2_subdev_state_get_interval(state, MAX9286_SRC_PAD);
> > > +	interval->numerator = 0;
> > > +	interval->denominator = 0;
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -1142,7 +1139,13 @@ 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);
> > > +
> > > +	/*
> > > +	 * Use automatic FRAMESYNC mode. FRAMESYNC is taken from the slowest
> > > +	 * link.
> > > +	 */
> > > +	max9286_write(priv, 0x01, MAX9286_FSYNCMODE_INT_HIZ |
> > > +				  MAX9286_FSYNCMETH_AUTO);
> > >
> > >  	cfg = max9286_read(priv, 0x1c);
> > >  	if (cfg < 0)
diff mbox series

Patch

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 7fad190cd9b3..6930a98c8965 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)
 {
+	const 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);
@@ -801,7 +802,7 @@  static int max9286_s_stream(struct v4l2_subdev *sd, int enable)
 		format = v4l2_subdev_state_get_format(state, MAX9286_SRC_PAD);
 
 		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
@@ -874,19 +875,11 @@  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;
+	interval->interval = *v4l2_subdev_state_get_interval(sd_state,
+							     interval->pad);
 
 	return 0;
 }
@@ -895,19 +888,11 @@  static int max9286_set_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;
 
-	priv->interval = interval->interval;
+	*v4l2_subdev_state_get_interval(sd_state,
+					interval->pad) = interval->interval;
 
 	return 0;
 }
@@ -993,9 +978,21 @@  static const struct v4l2_mbus_framefmt max9286_default_format = {
 static int max9286_init_state(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_state *state)
 {
+	struct v4l2_fract *interval;
+
 	for (unsigned int i = 0; i < MAX9286_N_PADS; i++)
 		*v4l2_subdev_state_get_format(state, i) = max9286_default_format;
 
+	/*
+	 * Special case: a null interval enables automatic FRAMESYNC mode.
+	 *
+	 * FRAMESYNC is taken from the slowest link. See register 0x01
+	 * configuration.
+	 */
+	interval = v4l2_subdev_state_get_interval(state, MAX9286_SRC_PAD);
+	interval->numerator = 0;
+	interval->denominator = 0;
+
 	return 0;
 }
 
@@ -1142,7 +1139,13 @@  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);
+
+	/*
+	 * Use automatic FRAMESYNC mode. FRAMESYNC is taken from the slowest
+	 * link.
+	 */
+	max9286_write(priv, 0x01, MAX9286_FSYNCMODE_INT_HIZ |
+				  MAX9286_FSYNCMETH_AUTO);
 
 	cfg = max9286_read(priv, 0x1c);
 	if (cfg < 0)