[PATCH 0/2] media: ov7670: fix regressions caused by "hook s_power onto v4l2 core"

[PATCH 0/2] media: ov7670: fix regressions caused by "hook s_power onto v4l2 core"

[Akinobu Mita]

This patchset fixes the problems introduced by recent change to ov7670.

Akinobu Mita (2):
  media: ov7670: restore default settings after power-up
  media: ov7670: don't access registers when the device is powered off

 drivers/media/i2c/ov7670.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

Cc: Lubomir Rintel <lkundrak@v3.sk>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
-- 
2.7.4

[PATCH 1/2] media: ov7670: restore default settings after power-up

[Akinobu Mita]

Since commit 3d6a8fe25605 ("media: ov7670: hook s_power onto v4l2 core"),
the device is actually powered off while the video stream is stopped.

The frame format and framerate are restored right after power-up, but
restoring the default register settings is forgotten.

Fixes: 3d6a8fe25605 ("media: ov7670: hook s_power onto v4l2 core")
Cc: Lubomir Rintel <lkundrak@v3.sk>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/media/i2c/ov7670.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index a7d26b2..e65693c 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1664,6 +1664,7 @@ static int ov7670_s_power(struct v4l2_subdev *sd, int on)
 
 	if (on) {
 		ov7670_power_on (sd);
+		ov7670_init(sd, 0);
 		ov7670_apply_fmt(sd);
 		ov7675_apply_framerate(sd);
 		v4l2_ctrl_handler_setup(&info->hdl);
-- 
2.7.4

[PATCH 2/2] media: ov7670: don't access registers when the device is powered off

[Akinobu Mita]

Since commit 3d6a8fe25605 ("media: ov7670: hook s_power onto v4l2 core"),
the device is actually powered off while the video stream is stopped.

So now set_format and s_frame_interval could be called while the device
is powered off, but these callbacks try to change the register settings
at this time.

The frame format and framerate will be restored right after power-up, so
we can just postpone applying these changes at these callbacks if the
device is not powered up.

Fixes: 3d6a8fe25605 ("media: ov7670: hook s_power onto v4l2 core")
Cc: Lubomir Rintel <lkundrak@v3.sk>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/media/i2c/ov7670.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e65693c..44c3eed 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -864,7 +864,15 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd,
 	/* Recalculate frame rate */
 	ov7675_get_framerate(sd, tpf);
 
-	return ov7675_apply_framerate(sd);
+	/*
+	 * If the device is not powered up by the host driver do
+	 * not apply any changes to H/W at this time. Instead
+	 * the framerate will be restored right after power-up.
+	 */
+	if (info->on)
+		return ov7675_apply_framerate(sd);
+
+	return 0;
 }
 
 static void ov7670_get_framerate_legacy(struct v4l2_subdev *sd,
@@ -895,7 +903,16 @@ static int ov7670_set_framerate_legacy(struct v4l2_subdev *sd,
 	info->clkrc = (info->clkrc & 0x80) | div;
 	tpf->numerator = 1;
 	tpf->denominator = info->clock_speed / div;
-	return ov7670_write(sd, REG_CLKRC, info->clkrc);
+
+	/*
+	 * If the device is not powered up by the host driver do
+	 * not apply any changes to H/W at this time. Instead
+	 * the framerate will be restored right after power-up.
+	 */
+	if (info->on)
+		return ov7670_write(sd, REG_CLKRC, info->clkrc);
+
+	return 0;
 }
 
 /*
@@ -1105,9 +1122,13 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
 	if (ret)
 		return ret;
 
-	ret = ov7670_apply_fmt(sd);
-	if (ret)
-		return ret;
+	/*
+	 * If the device is not powered up by the host driver do
+	 * not apply any changes to H/W at this time. Instead
+	 * the frame format will be restored right after power-up.
+	 */
+	if (info->on)
+		return ov7670_apply_fmt(sd);
 
 	return 0;
 }
-- 
2.7.4

Re: [PATCH 0/2] media: ov7670: fix regressions caused by "hook s_power onto v4l2 core"

[Lubomir Rintel]

On Tue, 2019-03-12 at 00:36 +0900, Akinobu Mita wrote:
> This patchset fixes the problems introduced by recent change to ov7670.
> 
> Akinobu Mita (2):
>   media: ov7670: restore default settings after power-up
>   media: ov7670: don't access registers when the device is powered off
> 
>  drivers/media/i2c/ov7670.c | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> Cc: Lubomir Rintel <lkundrak@v3.sk>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>

For the both patches in the set:

Reviewed-by: Lubomir Rintel <lkundrak@v3.sk>
Tested-by: Lubomir Rintel <lkundrak@v3.sk>

Thank you,
Lubo

Re: [PATCH 0/2] media: ov7670: fix regressions caused by "hook s_power onto v4l2 core"

[Sakari Ailus]

On Tue, Mar 12, 2019 at 06:16:08PM +0100, Lubomir Rintel wrote:
> On Tue, 2019-03-12 at 00:36 +0900, Akinobu Mita wrote:
> > This patchset fixes the problems introduced by recent change to ov7670.
> > 
> > Akinobu Mita (2):
> >   media: ov7670: restore default settings after power-up
> >   media: ov7670: don't access registers when the device is powered off
> > 
> >  drivers/media/i2c/ov7670.c | 32 +++++++++++++++++++++++++++-----
> >  1 file changed, 27 insertions(+), 5 deletions(-)
> > 
> > Cc: Lubomir Rintel <lkundrak@v3.sk>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> For the both patches in the set:
> 
> Reviewed-by: Lubomir Rintel <lkundrak@v3.sk>
> Tested-by: Lubomir Rintel <lkundrak@v3.sk>

Thanks, guys!

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH 0/2] media: ov7670: fix regressions caused by "hook s_power onto v4l2 core"

[Eugen.Hristev]

On 13.03.2019 23:05, Sakari Ailus wrote:

> On Tue, Mar 12, 2019 at 06:16:08PM +0100, Lubomir Rintel wrote:
>> On Tue, 2019-03-12 at 00:36 +0900, Akinobu Mita wrote:
>>> This patchset fixes the problems introduced by recent change to ov7670.
>>>
>>> Akinobu Mita (2):
>>>    media: ov7670: restore default settings after power-up
>>>    media: ov7670: don't access registers when the device is powered off
>>>
>>>   drivers/media/i2c/ov7670.c | 32 +++++++++++++++++++++++++++-----
>>>   1 file changed, 27 insertions(+), 5 deletions(-)
>>>
>>> Cc: Lubomir Rintel <lkundrak@v3.sk>
>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
>>
>> For the both patches in the set:
>>
>> Reviewed-by: Lubomir Rintel <lkundrak@v3.sk>
>> Tested-by: Lubomir Rintel <lkundrak@v3.sk>
> 
> Thanks, guys!
> 

Hi Akinobu,

I am having issues with this sensor, and your patches do not fix them 
for me ( maybe they are not supposed to )

My issues are like this: once I set a format and start streaming, if I 
stop streaming and reconfigure the format , for example YUYV after RAW, 
or RGB565 after RAW and viceversa, the sensor looks to have completely 
messed up settings: images obtained are very bad.
This did not happen for me with older kernel version (4.14 stable for 
example).
I can help with testing patches if you need.
My setup is with atmel-isc with parallel interface on board at91 
sama5d2_xplained

Hope this helps,

Eugen

Re: [PATCH 0/2] media: ov7670: fix regressions caused by "hook s_power onto v4l2 core"

[Akinobu Mita]

2019年3月19日(火) 18:14 <Eugen.Hristev@microchip.com>:
>
>
>
> On 13.03.2019 23:05, Sakari Ailus wrote:
>
> > On Tue, Mar 12, 2019 at 06:16:08PM +0100, Lubomir Rintel wrote:
> >> On Tue, 2019-03-12 at 00:36 +0900, Akinobu Mita wrote:
> >>> This patchset fixes the problems introduced by recent change to ov7670.
> >>>
> >>> Akinobu Mita (2):
> >>>    media: ov7670: restore default settings after power-up
> >>>    media: ov7670: don't access registers when the device is powered off
> >>>
> >>>   drivers/media/i2c/ov7670.c | 32 +++++++++++++++++++++++++++-----
> >>>   1 file changed, 27 insertions(+), 5 deletions(-)
> >>>
> >>> Cc: Lubomir Rintel <lkundrak@v3.sk>
> >>> Cc: Jonathan Corbet <corbet@lwn.net>
> >>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> >>
> >> For the both patches in the set:
> >>
> >> Reviewed-by: Lubomir Rintel <lkundrak@v3.sk>
> >> Tested-by: Lubomir Rintel <lkundrak@v3.sk>
> >
> > Thanks, guys!
> >
>
> Hi Akinobu,
>
> I am having issues with this sensor, and your patches do not fix them
> for me ( maybe they are not supposed to )
>
> My issues are like this: once I set a format and start streaming, if I
> stop streaming and reconfigure the format , for example YUYV after RAW,
> or RGB565 after RAW and viceversa, the sensor looks to have completely
> messed up settings: images obtained are very bad.
> This did not happen for me with older kernel version (4.14 stable for
> example).
> I can help with testing patches if you need.

I'd suggest identifying which commit introduced your problem.