When supporting Rpi Camera v2 Module on the RZ/G2E, found the driver had
some issues with rcar mipi-csi driver. The sesnosr never entered into LP-11
state.
The powerup sequence in the datasheet[1] shows the sensor entering into
LP-11 in streaming mode, so to fix this issue transitions are performed
from "standby -> streaming -> standby" in the probe().
With this commit the sensor is able to enter LP-11 mode during power up,
as expected by some CSI-2 controllers.
[1] https://publiclab.org/system/images/photos/000/023/294/original/
RASPBERRY_PI_CAMERA_V2_DATASHEET_IMX219PQH5_7.0.0_Datasheet_XXX.PDF
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/media/i2c/imx219.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index f1effb5a5f66..8b48e148f2d0 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -1171,6 +1171,7 @@ static int imx219_check_hwcfg(struct device *dev)
static int imx219_probe(struct i2c_client *client)
{
+ const struct imx219_reg_list *reg_list;
struct device *dev = &client->dev;
struct imx219 *imx219;
int ret;
@@ -1224,6 +1225,38 @@ static int imx219_probe(struct i2c_client *client)
/* Set default mode to max resolution */
imx219->mode = &supported_modes[0];
+ /* sensor doesn't enter to LP-11 state upon power up until and unless
+ * streaming is started, so upon power up set the default format and
+ * switch the modes: standby -> streaming -> standby
+ */
+ /* getting sensor out of sleep */
+ ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
+ IMX219_REG_VALUE_08BIT, IMX219_MODE_STANDBY);
+ if (ret < 0)
+ goto error_power_off;
+ usleep_range(100, 110);
+
+ reg_list = &imx219->mode->reg_list;
+ ret = imx219_write_regs(imx219, reg_list->regs, reg_list->num_of_regs);
+ if (ret) {
+ dev_err(&client->dev, "%s failed to default mode\n", __func__);
+ goto error_power_off;
+ }
+
+ /* getting sensor out of sleep */
+ ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
+ IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING);
+ if (ret < 0)
+ goto error_power_off;
+ usleep_range(100, 110);
+
+ /* put sensor back to standby mode */
+ ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
+ IMX219_REG_VALUE_08BIT, IMX219_MODE_STANDBY);
+ if (ret < 0)
+ goto error_power_off;
+ usleep_range(100, 110);
+
ret = imx219_init_controls(imx219);
if (ret)
goto error_power_off;
--
2.20.1
Hi Lad.
Thanks again for the patch.
On Fri, 28 Feb 2020 at 16:55, Lad Prabhakar <[email protected]> wrote:
>
> When supporting Rpi Camera v2 Module on the RZ/G2E, found the driver had
> some issues with rcar mipi-csi driver. The sesnosr never entered into LP-11
s/sesnosr/sensor
> state.
>
> The powerup sequence in the datasheet[1] shows the sensor entering into
> LP-11 in streaming mode, so to fix this issue transitions are performed
> from "standby -> streaming -> standby" in the probe().
>
> With this commit the sensor is able to enter LP-11 mode during power up,
> as expected by some CSI-2 controllers.
I guess I'm lucky that the CSI2 receiver I deal with doesn't care on this front.
The datasheet does seem to imply that the line is left in what appears
to be LP-00 after power up, but this feels like a huge amount of stuff
to do.
> [1] https://publiclab.org/system/images/photos/000/023/294/original/
> RASPBERRY_PI_CAMERA_V2_DATASHEET_IMX219PQH5_7.0.0_Datasheet_XXX.PDF
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> drivers/media/i2c/imx219.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index f1effb5a5f66..8b48e148f2d0 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -1171,6 +1171,7 @@ static int imx219_check_hwcfg(struct device *dev)
>
> static int imx219_probe(struct i2c_client *client)
> {
> + const struct imx219_reg_list *reg_list;
> struct device *dev = &client->dev;
> struct imx219 *imx219;
> int ret;
> @@ -1224,6 +1225,38 @@ static int imx219_probe(struct i2c_client *client)
> /* Set default mode to max resolution */
> imx219->mode = &supported_modes[0];
>
> + /* sensor doesn't enter to LP-11 state upon power up until and unless
Remove "to"
> + * streaming is started, so upon power up set the default format and
> + * switch the modes: standby -> streaming -> standby
> + */
> + /* getting sensor out of sleep */
> + ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
> + IMX219_REG_VALUE_08BIT, IMX219_MODE_STANDBY);
The datasheet says the default for IMX219_REG_MODE_SELECT is already 0
/ STANDY, so this should be unnecessary as we've only just powered up.
> + if (ret < 0)
> + goto error_power_off;
> + usleep_range(100, 110);
> +
> + reg_list = &imx219->mode->reg_list;
> + ret = imx219_write_regs(imx219, reg_list->regs, reg_list->num_of_regs);
> + if (ret) {
> + dev_err(&client->dev, "%s failed to default mode\n", __func__);
> + goto error_power_off;
> + }
Seeing as we don't want the images produced, and we're about to power
the sensor back down again, do the default register settings do enough
to allow the shift to LP-11? ie can we drop writing any mode setup
registers here, and just got to STREAMING and back to STANDBY?
> + /* getting sensor out of sleep */
We already did that above. This is standby->streaming.
> + ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
> + IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING);
> + if (ret < 0)
> + goto error_power_off;
> + usleep_range(100, 110);
> +
> + /* put sensor back to standby mode */
> + ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
> + IMX219_REG_VALUE_08BIT, IMX219_MODE_STANDBY);
> + if (ret < 0)
> + goto error_power_off;
> + usleep_range(100, 110);
> +
> ret = imx219_init_controls(imx219);
> if (ret)
> goto error_power_off;
> --
> 2.20.1
Cheers,
Dave
Hi Dave,
Thank you for the review.
On Mon, Mar 2, 2020 at 3:24 PM Dave Stevenson
<[email protected]> wrote:
>
> Hi Lad.
>
> Thanks again for the patch.
>
> On Fri, 28 Feb 2020 at 16:55, Lad Prabhakar <[email protected]> wrote:
> >
> > When supporting Rpi Camera v2 Module on the RZ/G2E, found the driver had
> > some issues with rcar mipi-csi driver. The sesnosr never entered into LP-11
>
> s/sesnosr/sensor
>
my bad shall fix that.
> > state.
> >
> > The powerup sequence in the datasheet[1] shows the sensor entering into
> > LP-11 in streaming mode, so to fix this issue transitions are performed
> > from "standby -> streaming -> standby" in the probe().
> >
> > With this commit the sensor is able to enter LP-11 mode during power up,
> > as expected by some CSI-2 controllers.
>
> I guess I'm lucky that the CSI2 receiver I deal with doesn't care on this front.
> The datasheet does seem to imply that the line is left in what appears
> to be LP-00 after power up, but this feels like a huge amount of stuff
> to do.
>
> > [1] https://publiclab.org/system/images/photos/000/023/294/original/
> > RASPBERRY_PI_CAMERA_V2_DATASHEET_IMX219PQH5_7.0.0_Datasheet_XXX.PDF
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > drivers/media/i2c/imx219.c | 33 +++++++++++++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> > index f1effb5a5f66..8b48e148f2d0 100644
> > --- a/drivers/media/i2c/imx219.c
> > +++ b/drivers/media/i2c/imx219.c
> > @@ -1171,6 +1171,7 @@ static int imx219_check_hwcfg(struct device *dev)
> >
> > static int imx219_probe(struct i2c_client *client)
> > {
> > + const struct imx219_reg_list *reg_list;
> > struct device *dev = &client->dev;
> > struct imx219 *imx219;
> > int ret;
> > @@ -1224,6 +1225,38 @@ static int imx219_probe(struct i2c_client *client)
> > /* Set default mode to max resolution */
> > imx219->mode = &supported_modes[0];
> >
> > + /* sensor doesn't enter to LP-11 state upon power up until and unless
>
> Remove "to"
>
will do.
> > + * streaming is started, so upon power up set the default format and
> > + * switch the modes: standby -> streaming -> standby
> > + */
> > + /* getting sensor out of sleep */
> > + ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
> > + IMX219_REG_VALUE_08BIT, IMX219_MODE_STANDBY);
>
> The datasheet says the default for IMX219_REG_MODE_SELECT is already 0
> / STANDY, so this should be unnecessary as we've only just powered up.
>
Agreed.
> > + if (ret < 0)
> > + goto error_power_off;
> > + usleep_range(100, 110);
> > +
> > + reg_list = &imx219->mode->reg_list;
> > + ret = imx219_write_regs(imx219, reg_list->regs, reg_list->num_of_regs);
> > + if (ret) {
> > + dev_err(&client->dev, "%s failed to default mode\n", __func__);
> > + goto error_power_off;
> > + }
>
> Seeing as we don't want the images produced, and we're about to power
> the sensor back down again, do the default register settings do enough
> to allow the shift to LP-11? ie can we drop writing any mode setup
> registers here, and just got to STREAMING and back to STANDBY?
>
Yes shall replace the sequence.
> > + /* getting sensor out of sleep */
>
> We already did that above. This is standby->streaming.
>
agreed.
> > + ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
> > + IMX219_REG_VALUE_08BIT, IMX219_MODE_STREAMING);
> > + if (ret < 0)
> > + goto error_power_off;
> > + usleep_range(100, 110);
> > +
> > + /* put sensor back to standby mode */
> > + ret = imx219_write_reg(imx219, IMX219_REG_MODE_SELECT,
> > + IMX219_REG_VALUE_08BIT, IMX219_MODE_STANDBY);
> > + if (ret < 0)
> > + goto error_power_off;
> > + usleep_range(100, 110);
> > +
Just the above two write_reg's should do the trick.
Cheers,
--Prabhakar
> > ret = imx219_init_controls(imx219);
> > if (ret)
> > goto error_power_off;
> > --
> > 2.20.1
>
> Cheers,
> Dave