Hi Sakari, On Thu, Jan 07, 2021 at 12:59:24AM +0200, Sakari Ailus wrote: > On Tue, Jan 05, 2021 at 05:28:45PM +0200, Laurent Pinchart wrote: > > The Ths_settle timing parameter depends solely on the pixel rate of the > > source. Calculate it at runtime instead of requiring it to be specified > > in the device tree. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > --- > > drivers/staging/media/imx/imx7-mipi-csis.c | 65 ++++++++++++++++++---- > > 1 file changed, 53 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c > > index 3c68ee8b2a59..c83450ac37fa 100644 > > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > > @@ -24,6 +24,7 @@ > > #include <linux/reset.h> > > #include <linux/spinlock.h> > > > > +#include <media/v4l2-ctrls.h> > > #include <media/v4l2-device.h> > > #include <media/v4l2-fwnode.h> > > #include <media/v4l2-mc.h> > > @@ -233,7 +234,11 @@ struct csi_state { > > struct media_pad pads[CSIS_PADS_NUM]; > > struct v4l2_subdev mipi_sd; > > struct v4l2_async_notifier notifier; > > - struct v4l2_subdev *src_sd; > > + > > + struct { > > + struct v4l2_subdev *sd; > > + struct v4l2_ctrl *pixel_rate; > > + } src; > > > > u8 index; > > struct platform_device *pdev; > > @@ -482,6 +487,31 @@ static void __mipi_csis_set_format(struct csi_state *state) > > mipi_csis_write(state, MIPI_CSIS_ISPRESOL_CH0, val); > > } > > > > +static int mipi_csis_calculate_params(struct csi_state *state) > > +{ > > + u64 pixel_rate; > > + u32 lane_rate; > > + > > + /* Calculate the line rate from the pixel rate. */ > > + pixel_rate = v4l2_ctrl_g_ctrl_int64(state->src.pixel_rate); > > Could you instead use v4l2_get_link_freq()? > > I guess we're also moving to the LINK_FREQ control to tell this. I've developed this patch before we discussed moving to LINQ_FREQ :-) I'll fix this in v2. > > + lane_rate = div_u64(pixel_rate, state->bus.num_data_lanes) > > + * state->csis_fmt->width; > > + if (lane_rate < 80000000 || lane_rate > 1500000000) { > > + dev_dbg(state->dev, "Out-of-bound lane rate %u\n", lane_rate); > > + return -EINVAL; > > + } > > + > > + /* > > + * The HSSETTLE counter value is document in a table, but can also > > + * easily be calculated. > > + */ > > + state->hs_settle = (lane_rate - 5000000) / 45000000; > > Much better, thank you! -- Regards, Laurent Pinchart