Re: [PATCH v2] media: ov5640: do not change mode if format or frame interval is unchanged

2018-09-01 Thread Sakari Ailus 
On Thu, Aug 16, 2018 at 09:56:13AM +, Hugues FRUCHET wrote:
> Hi all,
> 
> Please ignore this v2, the v1 was merged.
> I've just pushed a new patch which fixes the regression observed, see:
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg134413.html
> 
> Sorry for inconvenience.

No worries; thanks for the fix!

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v2] media: ov5640: do not change mode if format or frame interval is unchanged

2018-08-16 Thread Hugues FRUCHET 
Hi all,

Please ignore this v2, the v1 was merged.
I've just pushed a new patch which fixes the regression observed, see:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg134413.html

Sorry for inconvenience.

Best regards,
Hugues.

On 08/13/2018 11:21 AM, Hugues Fruchet wrote:
> Save load of mode registers array when V4L2 client sets a format or a
> frame interval which selects the same mode than the current one.
> 
> Signed-off-by: Hugues Fruchet 
> ---
> Version 2:
>- Fix fuzzy image because of JPEG default format not being
>  changed according to new format selected, fix this.
>- Init sequence initialises format to YUV422 UYVY but
>  sensor->fmt initial value was set to JPEG, fix this.
> 
> 
>   drivers/media/i2c/ov5640.c | 33 -
>   1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 1ecbb7a..2ddd86d 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -223,6 +223,7 @@ struct ov5640_dev {
>   int power_count;
>   
>   struct v4l2_mbus_framefmt fmt;
> + bool pending_fmt_change;
>   
>   const struct ov5640_mode_info *current_mode;
>   enum ov5640_frame_rate current_fr;
> @@ -255,7 +256,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
> v4l2_ctrl *ctrl)
>* should be identified and removed to speed register load time
>* over i2c.
>*/
> -
> +/* YUV422 UYVY VGA@30fps */
>   static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
>   {0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
>   {0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
> @@ -1966,9 +1967,14 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
>   goto out;
>   }
>   
> - sensor->current_mode = new_mode;
> - sensor->fmt = *mbus_fmt;
> - sensor->pending_mode_change = true;
> + if (new_mode != sensor->current_mode) {
> + sensor->current_mode = new_mode;
> + sensor->pending_mode_change = true;
> + }
> + if (mbus_fmt->code != sensor->fmt.code) {
> + sensor->fmt = *mbus_fmt;
> + sensor->pending_fmt_change = true;
> + }
>   out:
>   mutex_unlock(&sensor->lock);
>   return ret;
> @@ -2508,8 +2514,10 @@ static int ov5640_s_frame_interval(struct v4l2_subdev 
> *sd,
>   goto out;
>   }
>   
> - sensor->current_mode = mode;
> - sensor->pending_mode_change = true;
> + if (mode != sensor->current_mode) {
> + sensor->current_mode = mode;
> + sensor->pending_mode_change = true;
> + }
>   out:
>   mutex_unlock(&sensor->lock);
>   return ret;
> @@ -2540,10 +2548,13 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, 
> int enable)
>   ret = ov5640_set_mode(sensor, sensor->current_mode);
>   if (ret)
>   goto out;
> + }
>   
> + if (enable && sensor->pending_fmt_change) {
>   ret = ov5640_set_framefmt(sensor, &sensor->fmt);
>   if (ret)
>   goto out;
> + sensor->pending_fmt_change = false;
>   }
>   
>   if (sensor->ep.bus_type == V4L2_MBUS_CSI2)
> @@ -2638,9 +2649,14 @@ static int ov5640_probe(struct i2c_client *client,
>   return -ENOMEM;
>   
>   sensor->i2c_client = client;
> +
> + /*
> +  * default init sequence initialize sensor to
> +  * YUV422 UYVY VGA@30fps
> +  */
>   fmt = &sensor->fmt;
> - fmt->code = ov5640_formats[0].code;
> - fmt->colorspace = ov5640_formats[0].colorspace;
> + fmt->code = MEDIA_BUS_FMT_UYVY8_2X8;
> + fmt->colorspace = V4L2_COLORSPACE_SRGB;
>   fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
>   fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>   fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
> @@ -2652,7 +2668,6 @@ static int ov5640_probe(struct i2c_client *client,
>   sensor->current_fr = OV5640_30_FPS;
>   sensor->current_mode =
>   &ov5640_mode_data[OV5640_30_FPS][OV5640_MODE_VGA_640_480];
> - sensor->pending_mode_change = true;
>   
>   sensor->ae_target = 52;
>   
>

[PATCH v2] media: ov5640: do not change mode if format or frame interval is unchanged

2018-08-13 Thread Hugues Fruchet 
Save load of mode registers array when V4L2 client sets a format or a
frame interval which selects the same mode than the current one.

Signed-off-by: Hugues Fruchet 
---
Version 2:
  - Fix fuzzy image because of JPEG default format not being
changed according to new format selected, fix this.
  - Init sequence initialises format to YUV422 UYVY but
sensor->fmt initial value was set to JPEG, fix this.


 drivers/media/i2c/ov5640.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1ecbb7a..2ddd86d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -223,6 +223,7 @@ struct ov5640_dev {
int power_count;
 
struct v4l2_mbus_framefmt fmt;
+   bool pending_fmt_change;
 
const struct ov5640_mode_info *current_mode;
enum ov5640_frame_rate current_fr;
@@ -255,7 +256,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct 
v4l2_ctrl *ctrl)
  * should be identified and removed to speed register load time
  * over i2c.
  */
-
+/* YUV422 UYVY VGA@30fps */
 static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
{0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
@@ -1966,9 +1967,14 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
goto out;
}
 
-   sensor->current_mode = new_mode;
-   sensor->fmt = *mbus_fmt;
-   sensor->pending_mode_change = true;
+   if (new_mode != sensor->current_mode) {
+   sensor->current_mode = new_mode;
+   sensor->pending_mode_change = true;
+   }
+   if (mbus_fmt->code != sensor->fmt.code) {
+   sensor->fmt = *mbus_fmt;
+   sensor->pending_fmt_change = true;
+   }
 out:
mutex_unlock(&sensor->lock);
return ret;
@@ -2508,8 +2514,10 @@ static int ov5640_s_frame_interval(struct v4l2_subdev 
*sd,
goto out;
}
 
-   sensor->current_mode = mode;
-   sensor->pending_mode_change = true;
+   if (mode != sensor->current_mode) {
+   sensor->current_mode = mode;
+   sensor->pending_mode_change = true;
+   }
 out:
mutex_unlock(&sensor->lock);
return ret;
@@ -2540,10 +2548,13 @@ static int ov5640_s_stream(struct v4l2_subdev *sd, int 
enable)
ret = ov5640_set_mode(sensor, sensor->current_mode);
if (ret)
goto out;
+   }
 
+   if (enable && sensor->pending_fmt_change) {
ret = ov5640_set_framefmt(sensor, &sensor->fmt);
if (ret)
goto out;
+   sensor->pending_fmt_change = false;
}
 
if (sensor->ep.bus_type == V4L2_MBUS_CSI2)
@@ -2638,9 +2649,14 @@ static int ov5640_probe(struct i2c_client *client,
return -ENOMEM;
 
sensor->i2c_client = client;
+
+   /*
+* default init sequence initialize sensor to
+* YUV422 UYVY VGA@30fps
+*/
fmt = &sensor->fmt;
-   fmt->code = ov5640_formats[0].code;
-   fmt->colorspace = ov5640_formats[0].colorspace;
+   fmt->code = MEDIA_BUS_FMT_UYVY8_2X8;
+   fmt->colorspace = V4L2_COLORSPACE_SRGB;
fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
@@ -2652,7 +2668,6 @@ static int ov5640_probe(struct i2c_client *client,
sensor->current_fr = OV5640_30_FPS;
sensor->current_mode =
&ov5640_mode_data[OV5640_30_FPS][OV5640_MODE_VGA_640_480];
-   sensor->pending_mode_change = true;
 
sensor->ae_target = 52;
 
-- 
2.7.4