Hi Laurent,
Thanks for the patchset.
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.
> + 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,
Sakari Ailus
