| Message ID | 20230530173000.3060865-13-dave.stevenson@raspberrypi.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | imx258 improvements series | expand |
Hi Dave On Tue, May 30, 2023 at 06:29:51PM +0100, Dave Stevenson wrote: > The sensor supports the clock lane either remaining in HS mode > during frame blanking, or dropping to LP11. > > Add configuration of the mode via V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK. Assuming a follow-up patch for DT Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > --- > drivers/media/i2c/imx258.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c > index 1fa83fe82f27..b5c2dcb7c9e6 100644 > --- a/drivers/media/i2c/imx258.c > +++ b/drivers/media/i2c/imx258.c > @@ -72,6 +72,8 @@ > /* Test Pattern Control */ > #define IMX258_REG_TEST_PATTERN 0x0600 > > +#define IMX258_CLK_BLANK_STOP 0x4040 > + > /* Orientation */ > #define REG_MIRROR_FLIP_CONTROL 0x0101 > #define REG_CONFIG_MIRROR_FLIP 0x03 > @@ -634,6 +636,7 @@ struct imx258 { > const struct imx258_link_freq_config *link_freq_configs; > const s64 *link_freq_menu_items; > unsigned int nlanes; > + unsigned int csi2_flags; > > /* > * Mutex for serialized access: > @@ -1072,6 +1075,15 @@ static int imx258_start_streaming(struct imx258 *imx258) > return ret; > } > > + ret = imx258_write_reg(imx258, IMX258_CLK_BLANK_STOP, > + IMX258_REG_VALUE_08BIT, > + imx258->csi2_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ? > + 1 : 0); > + if (ret) { > + dev_err(&client->dev, "%s failed to set clock lane mode\n", __func__); > + return ret; > + } > + > /* Apply default values of current mode */ > reg_list = &imx258->cur_mode->reg_list; > ret = imx258_write_regs(imx258, reg_list->regs, reg_list->num_of_regs); > @@ -1486,6 +1498,8 @@ static int imx258_probe(struct i2c_client *client) > goto error_endpoint_poweron; > } > > + imx258->csi2_flags = ep.bus.mipi_csi2.flags; > + > /* Initialize subdev */ > v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops); > > -- > 2.25.1 >
Hi Jacopo On Fri, 2 Jun 2023 at 14:27, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > Hi Dave > > On Tue, May 30, 2023 at 06:29:51PM +0100, Dave Stevenson wrote: > > The sensor supports the clock lane either remaining in HS mode > > during frame blanking, or dropping to LP11. > > > > Add configuration of the mode via V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK. > > Assuming a follow-up patch for DT > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> It's already covered in video-interfaces.yaml, and is an optional flag as the driver will work in either manner, so do the bindings need updating? In checking the default value for this register, it is 0x01, or non-continuous. The original omission of this property from the binding and driver therefore means that an existing binding will most likely have omitted it and be believing the sensor is in continuous clock mode when it isn't. Now that we check the mode requested, it will actually adopt continuous clock mode and may no longer work with the receiver. Perhaps it's best to drop this patch, and add it as a note to anyone preparing a talk on Camera Sensor Drivers Compliance ;-) Dave > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > --- > > drivers/media/i2c/imx258.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c > > index 1fa83fe82f27..b5c2dcb7c9e6 100644 > > --- a/drivers/media/i2c/imx258.c > > +++ b/drivers/media/i2c/imx258.c > > @@ -72,6 +72,8 @@ > > /* Test Pattern Control */ > > #define IMX258_REG_TEST_PATTERN 0x0600 > > > > +#define IMX258_CLK_BLANK_STOP 0x4040 > > + > > /* Orientation */ > > #define REG_MIRROR_FLIP_CONTROL 0x0101 > > #define REG_CONFIG_MIRROR_FLIP 0x03 > > @@ -634,6 +636,7 @@ struct imx258 { > > const struct imx258_link_freq_config *link_freq_configs; > > const s64 *link_freq_menu_items; > > unsigned int nlanes; > > + unsigned int csi2_flags; > > > > /* > > * Mutex for serialized access: > > @@ -1072,6 +1075,15 @@ static int imx258_start_streaming(struct imx258 *imx258) > > return ret; > > } > > > > + ret = imx258_write_reg(imx258, IMX258_CLK_BLANK_STOP, > > + IMX258_REG_VALUE_08BIT, > > + imx258->csi2_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ? > > + 1 : 0); > > + if (ret) { > > + dev_err(&client->dev, "%s failed to set clock lane mode\n", __func__); > > + return ret; > > + } > > + > > /* Apply default values of current mode */ > > reg_list = &imx258->cur_mode->reg_list; > > ret = imx258_write_regs(imx258, reg_list->regs, reg_list->num_of_regs); > > @@ -1486,6 +1498,8 @@ static int imx258_probe(struct i2c_client *client) > > goto error_endpoint_poweron; > > } > > > > + imx258->csi2_flags = ep.bus.mipi_csi2.flags; > > + > > /* Initialize subdev */ > > v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops); > > > > -- > > 2.25.1 > >
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c index 1fa83fe82f27..b5c2dcb7c9e6 100644 --- a/drivers/media/i2c/imx258.c +++ b/drivers/media/i2c/imx258.c @@ -72,6 +72,8 @@ /* Test Pattern Control */ #define IMX258_REG_TEST_PATTERN 0x0600 +#define IMX258_CLK_BLANK_STOP 0x4040 + /* Orientation */ #define REG_MIRROR_FLIP_CONTROL 0x0101 #define REG_CONFIG_MIRROR_FLIP 0x03 @@ -634,6 +636,7 @@ struct imx258 { const struct imx258_link_freq_config *link_freq_configs; const s64 *link_freq_menu_items; unsigned int nlanes; + unsigned int csi2_flags; /* * Mutex for serialized access: @@ -1072,6 +1075,15 @@ static int imx258_start_streaming(struct imx258 *imx258) return ret; } + ret = imx258_write_reg(imx258, IMX258_CLK_BLANK_STOP, + IMX258_REG_VALUE_08BIT, + imx258->csi2_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ? + 1 : 0); + if (ret) { + dev_err(&client->dev, "%s failed to set clock lane mode\n", __func__); + return ret; + } + /* Apply default values of current mode */ reg_list = &imx258->cur_mode->reg_list; ret = imx258_write_regs(imx258, reg_list->regs, reg_list->num_of_regs); @@ -1486,6 +1498,8 @@ static int imx258_probe(struct i2c_client *client) goto error_endpoint_poweron; } + imx258->csi2_flags = ep.bus.mipi_csi2.flags; + /* Initialize subdev */ v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
The sensor supports the clock lane either remaining in HS mode during frame blanking, or dropping to LP11. Add configuration of the mode via V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> --- drivers/media/i2c/imx258.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)