Re: [PATCH] staging: atomisp: add a driver for ov5648 camera sensor

2017-09-18 Thread Andy Shevchenko 
On Mon, 2017-09-11 at 20:03 +0200, Devid Antonio Filoni wrote:
> On Mon, Sep 11, 2017 at 05:55:29PM +0300, Sakari Ailus wrote:
> > Hi Devid,
> > 
> > Please see my comments below.
> > 
> > Andy: please look for "INT5648".
> 
> Hi Sakari,
> I'm replying below to your comments. I'll work on a v2 patch as soon
> as we get
> more comments.
> 
> About "INT5648", I extracted it from the DSDT of my Lenovo Miix 310,
> take a look
> at https://pastebin.com/ExHWYr8g .

First of all, thank you, Sakari, to raise a flag here.

Second, Devid, please answer to the following:
is it an official BIOS which is available in the wild?

If it's so, please, add a paragraph to the commit message explaining how do you 
get this and point to the DSDT excerpt.
Put an answer to above question to the commit message as well.

-- 
Andy Shevchenko 
Intel Finland Oy

Re: [PATCH] staging: atomisp: add a driver for ov5648 camera sensor

2017-09-11 Thread Devid Antonio Filoni 
On Mon, Sep 11, 2017 at 05:55:29PM +0300, Sakari Ailus wrote:
> Hi Devid,
> 
> Please see my comments below.
> 
> Andy: please look for "INT5648".

Hi Sakari,
I'm replying below to your comments. I'll work on a v2 patch as soon as we get
more comments.

About "INT5648", I extracted it from the DSDT of my Lenovo Miix 310, take a look
at https://pastebin.com/ExHWYr8g .

> 
> On Sun, Sep 10, 2017 at 02:23:07PM +0200, Devid Antonio Floni wrote:
> > The ov5680 5-megapixel camera sensor from OmniVision supports up to 
> > 2592x1944
> > resolution and MIPI CSI-2 interface. Output format is raw sRGB/Bayer with
> > 10 bits per colour (SGRBG10_1X10).
> > 
> > This patch is a port of ov5680 driver from following

Sorry, there is a typo here, the driver is for ov5648 (not ov5680).

> > 01org/ProductionKernelQuilts patches:
> > - 0004-ov2680-ov5648-Fork-lift-source-from-CTS.patch
> > - 0005-ov2680-ov5648-gminification.patch
> > - 0006-ov5648-Focus-support.patch
> > - 0007-Fix-resolution-issues-on-rear-camera.patch
> > - 0008-ov2680-ov5648-Enabled-the-set_exposure-functions.patch
> > - 0010-IRDA-cameras-mode-list-cleanup-unification.patch
> > - 0012-ov5648-Add-1296x972-binned-mode.patch
> > - 0014-ov5648-Adapt-to-Atomisp2-Gmin-VCM-framework.patch
> > - 0015-dw9714-Gmin-Atomisp-specific-VCM-driver.patch
> > - 0017-ov5648-Fix-deadlock-on-I2C-error.patch
> > - 0018-gc2155-Fix-deadlock-on-error.patch
> > - 0019-ov5648-Add-1280x960-binned-mode.patch
> > - 0020-ov5648-Make-1280x960-as-default-video-resolution.patch
> > - 0021-MALATA-Fix-testCameraToSurfaceTextureMetadata-CTS.patch
> > - 0023-OV5648-Added-5MP-video-resolution.patch
> > 
> > New changes introduced during the port:
> > - Rename entity types to entity functions
> > - Replace v4l2_subdev_fh by v4l2_subdev_pad_config
> > - Make use of media_bus_format enum
> > - Rename media_entity_init function to media_entity_pads_init
> > - Replace try_mbus_fmt by set_fmt
> > - Replace s_mbus_fmt by set_fmt
> > - Replace g_mbus_fmt by get_fmt
> > - Use s_ctrl/g_volatile_ctrl instead of ctrl core ops
> > - Update gmin platform API path
> > - Update and constify acpi_device_id
> > - Fix some checkpatch errors and warnings
> > - Remove FSF's mailing address from the sample GPL notice
> > 
> > I was not able to properly test this patch on my Lenovo Miix 310 due to 
> > other
> > issues with atomisp, the output is the same as ov2680 driver which is very 
> > similar.
> 
> Has this been somehow tested?

Nope. As far as I know my Lenovo Miix 310 and NextBook Flexx 10.1 use this 
camera sensor
however the driver doesn't work on both systems due to other issues in atomisp 
driver itself.
As I think this driver is identical to ov2680 I decided to send this patch, I 
also have a
patch for hm2056 camera sensor however I'm still not sending it as I'm not 
properly sure
about it.
Please note: my Lenovo Miix 310 has an ov2680 camera, this is how I know that 
ov2680 doesn't
work too (same errors). Let me know if this patch cannot be committed until 
atomisp is fixed.
Please take a look at https://bugzilla.kernel.org/show_bug.cgi?id=195877 for 
more infos.

> 
> > 
> > Signed-off-by: Devid Antonio Floni 
> > ---
> >  drivers/staging/media/atomisp/i2c/Kconfig  |   11 +
> >  drivers/staging/media/atomisp/i2c/Makefile |1 +
> >  drivers/staging/media/atomisp/i2c/ov5648.c | 1867 
> > 
> >  drivers/staging/media/atomisp/i2c/ov5648.h |  835 +
> >  4 files changed, 2714 insertions(+)
> >  create mode 100644 drivers/staging/media/atomisp/i2c/ov5648.c
> >  create mode 100644 drivers/staging/media/atomisp/i2c/ov5648.h
> > 
> > diff --git a/drivers/staging/media/atomisp/i2c/Kconfig 
> > b/drivers/staging/media/atomisp/i2c/Kconfig
> > index b80d29d..8ed2cf4 100644
> > --- a/drivers/staging/media/atomisp/i2c/Kconfig
> > +++ b/drivers/staging/media/atomisp/i2c/Kconfig
> > @@ -89,6 +89,17 @@ config VIDEO_OV2680
> >  
> >   It currently only works with the atomisp driver.
> >  
> > +config VIDEO_OV5648
> 
> It'd be good to have ATOMISP in the option as well (the others don't have
> it, I think I'll submit a patch) so that these won't collide with the real
> V4L2 subdev drivers. E.g. VIDEO_ATOMISP_OV5648.

Ok, this makes sense to me too.

> 
> > +   tristate "Omnivision OV5648 sensor support"
> > +   depends on I2C && VIDEO_V4L2
> > +   ---help---
> > + This is a Video4Linux2 sensor-level driver for the Omnivision
> > + OV5648 raw camera.
> > +
> > + ov5648 is a 5M raw sensor.
> > +
> > + It currently only works with the atomisp driver.
> > +
> 
> While it's nice to see a new sensor driver, it'd be even better if the
> atomisp driver was modified to work with V4L2 sub-device sensor drivers
> e.g. under drivers/media/i2c.
> 
> I'm not saying no to adding this specific driver as such though, this is
> staging... I wonder what others think.

The same can be done on other atomisp drivers as far as I

Re: [PATCH] staging: atomisp: add a driver for ov5648 camera sensor

2017-09-11 Thread Sakari Ailus 
Hi Devid,

Please see my comments below.

Andy: please look for "INT5648".

On Sun, Sep 10, 2017 at 02:23:07PM +0200, Devid Antonio Floni wrote:
> The ov5680 5-megapixel camera sensor from OmniVision supports up to 2592x1944
> resolution and MIPI CSI-2 interface. Output format is raw sRGB/Bayer with
> 10 bits per colour (SGRBG10_1X10).
> 
> This patch is a port of ov5680 driver from following
> 01org/ProductionKernelQuilts patches:
> - 0004-ov2680-ov5648-Fork-lift-source-from-CTS.patch
> - 0005-ov2680-ov5648-gminification.patch
> - 0006-ov5648-Focus-support.patch
> - 0007-Fix-resolution-issues-on-rear-camera.patch
> - 0008-ov2680-ov5648-Enabled-the-set_exposure-functions.patch
> - 0010-IRDA-cameras-mode-list-cleanup-unification.patch
> - 0012-ov5648-Add-1296x972-binned-mode.patch
> - 0014-ov5648-Adapt-to-Atomisp2-Gmin-VCM-framework.patch
> - 0015-dw9714-Gmin-Atomisp-specific-VCM-driver.patch
> - 0017-ov5648-Fix-deadlock-on-I2C-error.patch
> - 0018-gc2155-Fix-deadlock-on-error.patch
> - 0019-ov5648-Add-1280x960-binned-mode.patch
> - 0020-ov5648-Make-1280x960-as-default-video-resolution.patch
> - 0021-MALATA-Fix-testCameraToSurfaceTextureMetadata-CTS.patch
> - 0023-OV5648-Added-5MP-video-resolution.patch
> 
> New changes introduced during the port:
> - Rename entity types to entity functions
> - Replace v4l2_subdev_fh by v4l2_subdev_pad_config
> - Make use of media_bus_format enum
> - Rename media_entity_init function to media_entity_pads_init
> - Replace try_mbus_fmt by set_fmt
> - Replace s_mbus_fmt by set_fmt
> - Replace g_mbus_fmt by get_fmt
> - Use s_ctrl/g_volatile_ctrl instead of ctrl core ops
> - Update gmin platform API path
> - Update and constify acpi_device_id
> - Fix some checkpatch errors and warnings
> - Remove FSF's mailing address from the sample GPL notice
> 
> I was not able to properly test this patch on my Lenovo Miix 310 due to other
> issues with atomisp, the output is the same as ov2680 driver which is very 
> similar.

Has this been somehow tested?

> 
> Signed-off-by: Devid Antonio Floni 
> ---
>  drivers/staging/media/atomisp/i2c/Kconfig  |   11 +
>  drivers/staging/media/atomisp/i2c/Makefile |1 +
>  drivers/staging/media/atomisp/i2c/ov5648.c | 1867 
> 
>  drivers/staging/media/atomisp/i2c/ov5648.h |  835 +
>  4 files changed, 2714 insertions(+)
>  create mode 100644 drivers/staging/media/atomisp/i2c/ov5648.c
>  create mode 100644 drivers/staging/media/atomisp/i2c/ov5648.h
> 
> diff --git a/drivers/staging/media/atomisp/i2c/Kconfig 
> b/drivers/staging/media/atomisp/i2c/Kconfig
> index b80d29d..8ed2cf4 100644
> --- a/drivers/staging/media/atomisp/i2c/Kconfig
> +++ b/drivers/staging/media/atomisp/i2c/Kconfig
> @@ -89,6 +89,17 @@ config VIDEO_OV2680
>  
>   It currently only works with the atomisp driver.
>  
> +config VIDEO_OV5648

It'd be good to have ATOMISP in the option as well (the others don't have
it, I think I'll submit a patch) so that these won't collide with the real
V4L2 subdev drivers. E.g. VIDEO_ATOMISP_OV5648.

> +   tristate "Omnivision OV5648 sensor support"
> +   depends on I2C && VIDEO_V4L2
> +   ---help---
> + This is a Video4Linux2 sensor-level driver for the Omnivision
> + OV5648 raw camera.
> +
> + ov5648 is a 5M raw sensor.
> +
> + It currently only works with the atomisp driver.
> +

While it's nice to see a new sensor driver, it'd be even better if the
atomisp driver was modified to work with V4L2 sub-device sensor drivers
e.g. under drivers/media/i2c.

I'm not saying no to adding this specific driver as such though, this is
staging... I wonder what others think.

>  #
>  # Kconfig for flash drivers
>  #
> diff --git a/drivers/staging/media/atomisp/i2c/Makefile 
> b/drivers/staging/media/atomisp/i2c/Makefile
> index be13fab..26195d7 100644
> --- a/drivers/staging/media/atomisp/i2c/Makefile
> +++ b/drivers/staging/media/atomisp/i2c/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_VIDEO_MT9M114)+= mt9m114.o
>  obj-$(CONFIG_VIDEO_GC2235) += gc2235.o
>  obj-$(CONFIG_VIDEO_OV2722) += ov2722.o
>  obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
> +obj-$(CONFIG_VIDEO_OV5648) += ov5648.o
>  obj-$(CONFIG_VIDEO_GC0310) += gc0310.o
>  
>  obj-$(CONFIG_VIDEO_MSRLIST_HELPER) += libmsrlisthelper.o
> diff --git a/drivers/staging/media/atomisp/i2c/ov5648.c 
> b/drivers/staging/media/atomisp/i2c/ov5648.c
> new file mode 100644
> index 000..4815a19
> --- /dev/null
> +++ b/drivers/staging/media/atomisp/i2c/ov5648.c
> @@ -0,0 +1,1867 @@
> +/*
> + * Support for OmniVision OV5648 5M camera sensor.
> + * Based on OmniVision OV2722 driver.
> + *
> + * Copyright (c) 2013 Intel Corporation. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the

[PATCH] staging: atomisp: add a driver for ov5648 camera sensor

2017-09-10 Thread Devid Antonio Floni 
The ov5680 5-megapixel camera sensor from OmniVision supports up to 2592x1944
resolution and MIPI CSI-2 interface. Output format is raw sRGB/Bayer with
10 bits per colour (SGRBG10_1X10).

This patch is a port of ov5680 driver from following
01org/ProductionKernelQuilts patches:
- 0004-ov2680-ov5648-Fork-lift-source-from-CTS.patch
- 0005-ov2680-ov5648-gminification.patch
- 0006-ov5648-Focus-support.patch
- 0007-Fix-resolution-issues-on-rear-camera.patch
- 0008-ov2680-ov5648-Enabled-the-set_exposure-functions.patch
- 0010-IRDA-cameras-mode-list-cleanup-unification.patch
- 0012-ov5648-Add-1296x972-binned-mode.patch
- 0014-ov5648-Adapt-to-Atomisp2-Gmin-VCM-framework.patch
- 0015-dw9714-Gmin-Atomisp-specific-VCM-driver.patch
- 0017-ov5648-Fix-deadlock-on-I2C-error.patch
- 0018-gc2155-Fix-deadlock-on-error.patch
- 0019-ov5648-Add-1280x960-binned-mode.patch
- 0020-ov5648-Make-1280x960-as-default-video-resolution.patch
- 0021-MALATA-Fix-testCameraToSurfaceTextureMetadata-CTS.patch
- 0023-OV5648-Added-5MP-video-resolution.patch

New changes introduced during the port:
- Rename entity types to entity functions
- Replace v4l2_subdev_fh by v4l2_subdev_pad_config
- Make use of media_bus_format enum
- Rename media_entity_init function to media_entity_pads_init
- Replace try_mbus_fmt by set_fmt
- Replace s_mbus_fmt by set_fmt
- Replace g_mbus_fmt by get_fmt
- Use s_ctrl/g_volatile_ctrl instead of ctrl core ops
- Update gmin platform API path
- Update and constify acpi_device_id
- Fix some checkpatch errors and warnings
- Remove FSF's mailing address from the sample GPL notice

I was not able to properly test this patch on my Lenovo Miix 310 due to other
issues with atomisp, the output is the same as ov2680 driver which is very 
similar.

Signed-off-by: Devid Antonio Floni 
---
 drivers/staging/media/atomisp/i2c/Kconfig  |   11 +
 drivers/staging/media/atomisp/i2c/Makefile |1 +
 drivers/staging/media/atomisp/i2c/ov5648.c | 1867 
 drivers/staging/media/atomisp/i2c/ov5648.h |  835 +
 4 files changed, 2714 insertions(+)
 create mode 100644 drivers/staging/media/atomisp/i2c/ov5648.c
 create mode 100644 drivers/staging/media/atomisp/i2c/ov5648.h

diff --git a/drivers/staging/media/atomisp/i2c/Kconfig 
b/drivers/staging/media/atomisp/i2c/Kconfig
index b80d29d..8ed2cf4 100644
--- a/drivers/staging/media/atomisp/i2c/Kconfig
+++ b/drivers/staging/media/atomisp/i2c/Kconfig
@@ -89,6 +89,17 @@ config VIDEO_OV2680
 
  It currently only works with the atomisp driver.
 
+config VIDEO_OV5648
+   tristate "Omnivision OV5648 sensor support"
+   depends on I2C && VIDEO_V4L2
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the Omnivision
+ OV5648 raw camera.
+
+ ov5648 is a 5M raw sensor.
+
+ It currently only works with the atomisp driver.
+
 #
 # Kconfig for flash drivers
 #
diff --git a/drivers/staging/media/atomisp/i2c/Makefile 
b/drivers/staging/media/atomisp/i2c/Makefile
index be13fab..26195d7 100644
--- a/drivers/staging/media/atomisp/i2c/Makefile
+++ b/drivers/staging/media/atomisp/i2c/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_VIDEO_MT9M114)+= mt9m114.o
 obj-$(CONFIG_VIDEO_GC2235) += gc2235.o
 obj-$(CONFIG_VIDEO_OV2722) += ov2722.o
 obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
+obj-$(CONFIG_VIDEO_OV5648) += ov5648.o
 obj-$(CONFIG_VIDEO_GC0310) += gc0310.o
 
 obj-$(CONFIG_VIDEO_MSRLIST_HELPER) += libmsrlisthelper.o
diff --git a/drivers/staging/media/atomisp/i2c/ov5648.c 
b/drivers/staging/media/atomisp/i2c/ov5648.c
new file mode 100644
index 000..4815a19
--- /dev/null
+++ b/drivers/staging/media/atomisp/i2c/ov5648.c
@@ -0,0 +1,1867 @@
+/*
+ * Support for OmniVision OV5648 5M camera sensor.
+ * Based on OmniVision OV2722 driver.
+ *
+ * Copyright (c) 2013 Intel Corporation. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "../include/linux/atomisp_gmin_platform.h"
+
+#include "ov5648.h"
+
+#define OV5648_DEBUG_EN 0
+
+#define H_FLIP_DEFAULT 1
+#define V_FLIP_DEFAULT 0
+static int h_flag = H_FLIP_DEFAULT;
+static int v_flag = V_FLIP_DEFAULT;
+
+/* i2c read/write stuff */
+static int ov5648_read_reg(struct i2c_client *client,
+  u16 data_length, u16 reg, u16 *val)
+{
+   int err;
+   struct i2c_msg msg[2];
+   unsigned char data[6];
+
+   if