[PATCH] media: imx208: Add imx208 camera sensor driver

2018-07-10 Thread Ping-chung Chen
From: "Chen, Ping-chung" 

Add a V4L2 sub-device driver for the Sony IMX208 image sensor.
This is a camera sensor using the I2C bus for control and the
CSI-2 bus for data.

Signed-off-by: Ping-Chung Chen 
---
 drivers/media/i2c/Kconfig  |  11 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/imx208.c | 953 +
 3 files changed, 965 insertions(+)
 create mode 100644 drivers/media/i2c/imx208.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 8669853..1c739f4 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -595,6 +595,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_IMX208
+   tristate "Sony IMX208 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the Sony
+ IMX208 camera.
+
+  To compile this driver as a module, choose M here: the
+  module will be called imx208.
+
 config VIDEO_IMX258
tristate "Sony IMX258 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 837c428..47604c2 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -104,6 +104,7 @@ obj-$(CONFIG_VIDEO_I2C) += video-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
 obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
+obj-$(CONFIG_VIDEO_IMX208) += imx208.o
 obj-$(CONFIG_VIDEO_IMX258) += imx258.o
 obj-$(CONFIG_VIDEO_IMX274) += imx274.o
 
diff --git a/drivers/media/i2c/imx208.c b/drivers/media/i2c/imx208.c
new file mode 100644
index 000..9af9043
--- /dev/null
+++ b/drivers/media/i2c/imx208.c
@@ -0,0 +1,953 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#ifndef V4L2_CID_DIGITAL_GAIN
+#define V4L2_CID_DIGITAL_GAIN V4L2_CID_GAIN
+#endif
+
+#define IMX208_REG_VALUE_08BIT 1
+#define IMX208_REG_VALUE_16BIT 2
+#define IMX208_REG_VALUE_24BIT 3
+
+#define IMX208_REG_MODE_SELECT 0x0100
+#define IMX208_MODE_STANDBY0x00
+#define IMX208_MODE_STREAMING  0x01
+
+/* Chip ID */
+#define IMX208_REG_CHIP_ID 0x
+#define IMX208_CHIP_ID 0x0208
+
+/* V_TIMING internal */
+#define IMX208_REG_VTS 0x0340
+#define IMX208_VTS_60FPS   0x0472
+#define IMX208_VTS_BINNING 0x0239
+#define IMX208_VTS_60FPS_MIN   0x0458
+#define IMX208_VTS_BINNING_MIN 0x0230
+#define IMX208_VTS_MAX 0x
+
+/* HBLANK control - read only */
+#define IMX208_PPL_384MHZ  1124
+#define IMX208_PPL_96MHZ   1124
+
+/* Exposure control */
+#define IMX208_REG_EXPOSURE0x0202
+#define IMX208_EXPOSURE_MIN4
+#define IMX208_EXPOSURE_STEP   1
+#define IMX208_EXPOSURE_DEFAULT0x190
+#define IMX208_EXPOSURE_MAX65535
+
+/* Analog gain control */
+#define IMX208_REG_ANALOG_GAIN 0x0204
+#define IMX208_ANA_GAIN_MIN0
+#define IMX208_ANA_GAIN_MAX0x00e0
+#define IMX208_ANA_GAIN_STEP   1
+#define IMX208_ANA_GAIN_DEFAULT0x0
+
+/* Digital gain control */
+#define IMX208_REG_GR_DIGITAL_GAIN 0x020e
+#define IMX208_REG_R_DIGITAL_GAIN  0x0210
+#define IMX208_REG_B_DIGITAL_GAIN  0x0212
+#define IMX208_REG_GB_DIGITAL_GAIN 0x0214
+#define IMX208_DGTL_GAIN_MIN   0
+#define IMX208_DGTL_GAIN_MAX   4096   /* Max = 0xFFF */
+#define IMX208_DGTL_GAIN_DEFAULT   0x100
+#define IMX208_DGTL_GAIN_STEP   1
+
+/* Orientation */
+#define REG_MIRROR_FLIP_CONTROL0x0101
+#define REG_CONFIG_MIRROR_FLIP 0x03
+
+#define IMX208_REG_TEST_PATTERN_MODE   0x0600
+
+struct imx208_reg {
+   u16 address;
+   u8 val;
+};
+
+struct imx208_reg_list {
+   u32 num_of_regs;
+   const struct imx208_reg *regs;
+};
+
+/* Link frequency config */
+struct imx208_link_freq_config {
+   u32 pixels_per_line;
+
+   /* PLL registers for this link frequency */
+   struct imx208_reg_list reg_list;
+};
+
+/* Mode : resolution and related config&values */
+struct imx208_mode {
+   /* Frame width */
+   u32 width;
+   /* Frame height */
+   u32 height;
+
+   /* V-timing */
+   u32 vts_def;
+   u32 vts_min;
+
+   /* Index of Link frequency config to be used */
+   u32 link_freq_index;
+   /* Default register values */
+   struct imx208_reg_list reg_list;
+};
+
+static const struct imx208_reg mipi_data_rate[] = {
+   {0x0305, 0x02},
+   {0x0307, 0x50},
+   {0x303C, 0x3C},
+};
+
+static const struct imx208_reg mode_1936x1096_60fps_regs[] = {
+  

Re: [PATCH] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-10 Thread jacopo mondi
Hi Steve,
   thanks for testing!

On Mon, Jul 09, 2018 at 02:52:09PM -0700, Steve Longerbeam wrote:
> Hi Jacopo,
>
> I tested this patch on the i.MX6Q SabreSD with the OV5640 module.
> It fixes the LP-11 timeout at stream on, but the captured images
> are completely blank/black.

Intersting that the module was not starting up properly on other
(all?) i.MX6Q platforms, not only on Engicam's one. I didn't get this
initially.

Ok, so that's a step forward, but not enough probably. I'll keep
looking into this and get back!

Thanks
   j

>
> Steve
>
>
> On 07/06/2018 04:00 AM, Jacopo Mondi wrote:
> >From: Jacopo Mondi 
> >
> >Rework the MIPI interface startup sequence with the following changes:
> >
> >- Remove MIPI bus initialization from the initial settings blob
> >- At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in
> >   LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode.
> >- At s_stream time enable/disable the MIPI interface output.
> >- Restore default settings at set_power(0) time.
> >
> >Before this commit the sensor MIPI interface was initialized with settings
> >that require a start/stop sequence at power-up time in order to force lanes
> >into LP11 state, as they were initialized in LP00 when in 'sleep mode',
> >which is assumed to be the sensor manual definition for the D-PHY defined
> >stop mode.
> >
> >The stream start/stop was performed by enabling disabling clock gating,
> >and had the side effect to change the lanes sleep mode configuration when
> >stream was stopped.
> >
> >Clock gating/ungating:
> >-   ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5),
> >-on ? 0 : BIT(5));
> >-   if (ret)
> >
> >Set lanes in LP11 when in 'sleep mode':
> >-   ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
> >-  on ? 0x00 : 0x70);
> >
> >This commit fixes an issue reported by Jagan Teki on i.MX6 platforms that
> >prevents the host interface from powering up correctly:
> >https://lkml.org/lkml/2018/6/1/38
> >
> >It also improves MIPI capture operations stability on my testing platform
> >where MIPI capture often (silently) failed and returned all-purple frames.
> >
> >fixes: f22996db44e2 ("media: ov5640: add support of DVP parallel interface")
> >Reported-by: Jagan Teki 
> >Signed-off-by: Jacopo Mondi 
> >
> >---
> >
> >Hello,
> >   I'm sending this one as new patch instead of a v2 of the previously sent
> >series "media: i2c: ov5640: Re-work MIPI interface configuration" as the
> >previous one was not working on the Engicam i.Mx6 platform where Jagan
> >initially reported issues on.
> >
> >I've been able to test that capture now starts on said platform, but I've not
> >been able to visually verify any of the image content as I have no way yet to
> >transfer the raw images to my development host and verify their content 
> >(network
> >still not working for me on that platform :/ )
> >
> >On my other testing platform images are correct, but they already were with 
> >the
> >previous version of this patches too, so I assume the CSI-2 receiver is far 
> >more
> >tolerant there.
> >
> >Jagan, is there any way you could verify images? I would appreciate your
> >Tested-by tag in case they're correct.
> >
> >Also, as there seems to be a lot of people interested in ov5640 these days, I
> >have expanded the receivers list. Anyone that could give these patches a 
> >spin?
> >(ie. Sam reported issues too with the current MIPI startup sequence in a 
> >patch
> >series he shared on dropbox iirc...)
> >
> >Thanks
> >j
> >---
> >  drivers/media/i2c/ov5640.c | 91 
> > --
> >  1 file changed, 71 insertions(+), 20 deletions(-)
> >
> >diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> >index 1ecbb7a..7bbd1d7 100644
> >--- a/drivers/media/i2c/ov5640.c
> >+++ b/drivers/media/i2c/ov5640.c
> >@@ -286,10 +286,10 @@ static const struct reg_value 
> >ov5640_init_setting_30fps_VGA[] = {
> > {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
> > {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x3000, 0x00, 0, 0},
> > {0x3002, 0x1c, 0, 0}, {0x3004, 0xff, 0, 0}, {0x3006, 0xc3, 0, 0},
> >-{0x300e, 0x45, 0, 0}, {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
> >+{0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
> > {0x501f, 0x00, 0, 0}, {0x4713, 0x03, 0, 0}, {0x4407, 0x04, 0, 0},
> > {0x440e, 0x00, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
> >-{0x4837, 0x0a, 0, 0}, {0x4800, 0x04, 0, 0}, {0x3824, 0x02, 0, 0},
> >+{0x4837, 0x0a, 0, 0}, {0x3824, 0x02, 0, 0},
> > {0x5000, 0xa7, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x5180, 0xff, 0, 0},
> > {0x5181, 0xf2, 0, 0}, {0x5182, 0x00, 0, 0}, {0x5183, 0x14, 0, 0},
> > {0x5184, 0x25, 0, 0}, {0x5185, 0x24, 0, 0}, {0x5186, 0x09, 0, 0},
> >@@ -1102,12 +1102,18 @@ static int ov5640_set_stream_mipi(struct ov5640_dev 
> >*sensor, bool on)
> >  {
> > int ret;
> >
>

RE: [PATCH] media: imx208: Add imx208 camera sensor driver

2018-07-10 Thread Yeh, Andy
Hi PC,

Thanks for the patch.

Cc in Grant, and Intel Jim/Jason

> -Original Message-
> From: Chen, Ping-chung
> Sent: Tuesday, July 10, 2018 3:16 PM
> To: linux-media@vger.kernel.org
> Cc: sakari.ai...@linux.intel.com; Yeh, Andy ;
> tf...@chromium.org; Chen, Ping-chung 
> Subject: [PATCH] media: imx208: Add imx208 camera sensor driver
> +};
> +
> +static int imx208_enum_mbus_code(struct v4l2_subdev *sd,
> +   struct v4l2_subdev_pad_config *cfg,
> +   struct v4l2_subdev_mbus_code_enum *code) {
> + /* Only one bayer order(GRBG) is supported */
> + if (code->index > 0)
> + return -EINVAL;
> +

There is no limitation on using GRBG bayer order now. You can refer to imx355 
driver as well.

+static int imx355_enum_frame_size(struct v4l2_subdev *sd,
+  struct v4l2_subdev_pad_config *cfg,
+  struct v4l2_subdev_frame_size_enum 
*fse)
+{  
+   struct imx355 *imx355 = to_imx355(sd);

> + code->code = MEDIA_BUS_FMT_SRGGB10_1X10;
> +
> + return 0;
> +}
> +
> +static int imx208_enum_frame_size(struct v4l2_subdev *sd,
> +struct v4l2_subdev_pad_config *cfg,
> +struct v4l2_subdev_frame_size_enum *fse) {
> + if (fse->index >= ARRAY_SIZE(supported_modes))
> + return -EINVAL;
> +
> + if (fse->code != MEDIA_BUS_FMT_SRGGB10_1X10)
> + return -EINVAL;
> +
> + fse->min_width = supported_modes[fse->index].width;
> + fse->max_width = fse->min_width;
> + fse->min_height = supported_modes[fse->index].height;
> + fse->max_height = fse->min_height;
> +
> + return 0;
> +}
> +
> +static void imx208_update_pad_format(const struct imx208_mode *mode,
> +   struct v4l2_subdev_format *fmt) {
> + fmt->format.width = mode->width;
> + fmt->format.height = mode->height;
> + fmt->format.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> + fmt->format.field = V4L2_FIELD_NONE;
> +}
> +
> +
> +static int imx208_probe(struct i2c_client *client) {
> + struct imx208 *imx208;
> + int ret;
> + u32 val = 0;
> +
> + device_property_read_u32(&client->dev, "clock-frequency", &val);
> + if (val != 1920)
> + return -EINVAL;
> +
> + imx208 = devm_kzalloc(&client->dev, sizeof(*imx208), GFP_KERNEL);
> + if (!imx208)
> + return -ENOMEM;
> +
> + /* Initialize subdev */
> + v4l2_i2c_subdev_init(&imx208->sd, client, &imx208_subdev_ops);
> +
> + /* Check module identity */
> + ret = imx208_identify_module(imx208);
> + if (ret)
> + return ret;
> +
> + /* Set default mode to max resolution */
> + imx208->cur_mode = &supported_modes[0];
> +
> + ret = imx208_init_controls(imx208);
> + if (ret)
> + return ret;
> +
> + /* Initialize subdev */
> + imx208->sd.internal_ops = &imx208_internal_ops;
> + imx208->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +
> + /* Initialize source pad */
> + imx208->pad.flags = MEDIA_PAD_FL_SOURCE;

Please refer to IMX355 to change below code to new API on upstreamed kernel.
https://patchwork.linuxtv.org/patch/50028/

+   /* Initialize subdev */
+   imx355->sd.internal_ops = &imx355_internal_ops;
+   imx355->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
+   V4L2_SUBDEV_FL_HAS_EVENTS;
+   imx355->sd.entity.ops = &imx355_subdev_entity_ops;
+   imx355->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+   ret = media_entity_pads_init(&imx355->sd.entity, 1, 
&imx355->pad);


> + ret = media_entity_init(&imx208->sd.entity, 1, &imx208->pad, 0);
> + if (ret) {
> + dev_err(&client->dev, "%s failed:%d\n", __func__, ret);
> + goto error_handler_free;
> + }
> +
> + ret = v4l2_async_register_subdev_sensor_common(&imx208->sd);
> + if (ret < 0)
> + goto error_media_entity;
> +
> + pm_runtime_set_active(&client->dev);
> + pm_runtime_enable(&client->dev);
> + pm_runtime_idle(&client->dev);
> +
> + return 0;
> +
> +error_media_entity:
> + media_entity_cleanup(&imx208->sd.entity);
> +
> +error_handler_free:
> + imx208_free_controls(imx208);
> +
> + return ret;
> +}
> +
> +static int imx208_remove(struct i2c_client *client) {
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct imx208 *imx208 = to_imx208(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
> + imx208_free_controls(imx208);
> +
> + pm_runtime_disable(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops imx208_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS

[PATCHv6 01/12] media: add 'index' to struct media_v2_pad

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

The v2 pad structure never exposed the pad index, which made it impossible
to call the MEDIA_IOC_SETUP_LINK ioctl, which needs that information.

It is really trivial to just expose this information, so implement this.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 drivers/media/media-device.c |  1 +
 include/uapi/linux/media.h   | 12 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 47bb2254fbfd..047d38372a27 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -331,6 +331,7 @@ static long media_device_get_topology(struct media_device 
*mdev, void *arg)
kpad.id = pad->graph_obj.id;
kpad.entity_id = pad->entity->graph_obj.id;
kpad.flags = pad->flags;
+   kpad.index = pad->index;
 
if (copy_to_user(upad, &kpad, sizeof(kpad)))
ret = -EFAULT;
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 86c7dcc9cba3..f6338bd57929 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -305,11 +305,21 @@ struct media_v2_interface {
};
 } __attribute__ ((packed));
 
+/*
+ * Appeared in 4.19.0.
+ *
+ * The media_version argument comes from the media_version field in
+ * struct media_device_info.
+ */
+#define MEDIA_V2_PAD_HAS_INDEX(media_version) \
+   ((media_version) >= ((4 << 16) | (19 << 8) | 0))
+
 struct media_v2_pad {
__u32 id;
__u32 entity_id;
__u32 flags;
-   __u32 reserved[5];
+   __u32 index;
+   __u32 reserved[4];
 } __attribute__ ((packed));
 
 struct media_v2_link {
-- 
2.18.0



[PATCHv6 07/12] media.h: reorder video en/decoder functions

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

Keep the function defines in numerical order: 0x6000 comes after
0x2000, so move it back.

Signed-off-by: Hans Verkuil 
Reviewed-by: Laurent Pinchart 
---
 include/uapi/linux/media.h | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 6f594fa238c2..76d9bd64c116 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -89,13 +89,6 @@ struct media_device_info {
 #define MEDIA_ENT_F_FLASH  (MEDIA_ENT_F_OLD_SUBDEV_BASE + 
2)
 #define MEDIA_ENT_F_LENS   (MEDIA_ENT_F_OLD_SUBDEV_BASE + 
3)
 
-/*
- * Video decoder/encoder functions
- */
-#define MEDIA_ENT_F_ATV_DECODER
(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
-#define MEDIA_ENT_F_DV_DECODER (MEDIA_ENT_F_BASE + 0x6001)
-#define MEDIA_ENT_F_DV_ENCODER (MEDIA_ENT_F_BASE + 0x6002)
-
 /*
  * Digital TV, analog TV, radio and/or software defined radio tuner functions.
  *
@@ -140,6 +133,13 @@ struct media_device_info {
 #define MEDIA_ENT_F_VID_MUX(MEDIA_ENT_F_BASE + 0x5001)
 #define MEDIA_ENT_F_VID_IF_BRIDGE  (MEDIA_ENT_F_BASE + 0x5002)
 
+/*
+ * Video decoder/encoder functions
+ */
+#define MEDIA_ENT_F_ATV_DECODER
(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
+#define MEDIA_ENT_F_DV_DECODER (MEDIA_ENT_F_BASE + 0x6001)
+#define MEDIA_ENT_F_DV_ENCODER (MEDIA_ENT_F_BASE + 0x6002)
+
 /* Entity flags */
 #define MEDIA_ENT_FL_DEFAULT   (1 << 0)
 #define MEDIA_ENT_FL_CONNECTOR (1 << 1)
-- 
2.18.0



[PATCHv6 04/12] media-ioc-g-topology.rst: document new 'flags' field

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

Document the new struct media_v2_entity 'flags' field.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 .../media/uapi/mediactl/media-ioc-g-topology.rst   | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index bae2b4db89cc..e572dd0d806d 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -142,7 +142,15 @@ desired arrays with the media graph elements.
-  Entity main function, see :ref:`media-entity-functions` for details.
 
 *  -  __u32
-   -  ``reserved``\ [6]
+   -  ``flags``
+   -  Entity flags, see :ref:`media-entity-flag` for details.
+ Only valid if ``MEDIA_V2_ENTITY_HAS_FLAGS(media_version)``
+ returns true. The ``media_version`` is defined in struct
+ :c:type:`media_device_info` and can be retrieved using
+ :ref:`MEDIA_IOC_DEVICE_INFO`.
+
+*  -  __u32
+   -  ``reserved``\ [5]
-  Reserved for future extensions. Drivers and applications must set
  this array to zero.
 
-- 
2.18.0



[PATCHv6 09/12] adv7180/tvp514x/tvp7002: fix entity function

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

The entity function was ORed with the flags field instead of
assigned to the function field. Correct this.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/i2c/adv7180.c | 2 +-
 drivers/media/i2c/tvp514x.c | 2 +-
 drivers/media/i2c/tvp7002.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 25d24a3f10a7..a727d7f806a1 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -1335,7 +1335,7 @@ static int adv7180_probe(struct i2c_client *client,
goto err_unregister_vpp_client;
 
state->pad.flags = MEDIA_PAD_FL_SOURCE;
-   sd->entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+   sd->entity.function = MEDIA_ENT_F_ATV_DECODER;
ret = media_entity_pads_init(&sd->entity, 1, &state->pad);
if (ret)
goto err_free_ctrl;
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 6a9890531d01..675b9ae212ab 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -1084,7 +1084,7 @@ tvp514x_probe(struct i2c_client *client, const struct 
i2c_device_id *id)
 #if defined(CONFIG_MEDIA_CONTROLLER)
decoder->pad.flags = MEDIA_PAD_FL_SOURCE;
decoder->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-   decoder->sd.entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+   decoder->sd.entity.function = MEDIA_ENT_F_ATV_DECODER;
 
ret = media_entity_pads_init(&decoder->sd.entity, 1, &decoder->pad);
if (ret < 0) {
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index 4599b7e28a8d..4f5c627579c7 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -1010,7 +1010,7 @@ static int tvp7002_probe(struct i2c_client *c, const 
struct i2c_device_id *id)
 #if defined(CONFIG_MEDIA_CONTROLLER)
device->pad.flags = MEDIA_PAD_FL_SOURCE;
device->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
-   device->sd.entity.flags |= MEDIA_ENT_F_ATV_DECODER;
+   device->sd.entity.function = MEDIA_ENT_F_ATV_DECODER;
 
error = media_entity_pads_init(&device->sd.entity, 1, &device->pad);
if (error < 0)
-- 
2.18.0



[PATCHv6 06/12] media.h: add MEDIA_ENT_F_DV_ENCODER

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

Add a new function for digital video encoders such as HDMI transmitters.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
Reviewed-by: Laurent Pinchart 
---
 Documentation/media/uapi/mediactl/media-types.rst | 7 +++
 include/uapi/linux/media.h| 3 ++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-types.rst 
b/Documentation/media/uapi/mediactl/media-types.rst
index c11b0c7e890b..e90d4d0a7f8b 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -206,6 +206,13 @@ Types and flags used to represent the media graph elements
  and output it in some digital video standard, with appropriate
  timing signals.
 
+*  -  ``MEDIA_ENT_F_DV_ENCODER``
+   -  Digital video encoder. The basic function of the video encoder is
+ to accept digital video from some digital video standard with
+ appropriate timing signals (usually a parallel video bus with sync
+ signals) and output this to a digital video output connector such
+ as HDMI or DisplayPort.
+
 ..  tabularcolumns:: |p{5.5cm}|p{12.0cm}|
 
 .. _media-entity-flag:
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 99f5e0978ebb..6f594fa238c2 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -90,10 +90,11 @@ struct media_device_info {
 #define MEDIA_ENT_F_LENS   (MEDIA_ENT_F_OLD_SUBDEV_BASE + 
3)
 
 /*
- * Video decoder functions
+ * Video decoder/encoder functions
  */
 #define MEDIA_ENT_F_ATV_DECODER
(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
 #define MEDIA_ENT_F_DV_DECODER (MEDIA_ENT_F_BASE + 0x6001)
+#define MEDIA_ENT_F_DV_ENCODER (MEDIA_ENT_F_BASE + 0x6002)
 
 /*
  * Digital TV, analog TV, radio and/or software defined radio tuner functions.
-- 
2.18.0



[PATCHv6 02/12] media-ioc-g-topology.rst: document new 'index' field

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

Document the new struct media_v2_pad 'index' field.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 .../media/uapi/mediactl/media-ioc-g-topology.rst | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index a3f259f83b25..bae2b4db89cc 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -176,7 +176,7 @@ desired arrays with the media graph elements.
 *  -  struct media_v2_intf_devnode
-  ``devnode``
-  Used only for device node interfaces. See
- :c:type:`media_v2_intf_devnode` for details..
+ :c:type:`media_v2_intf_devnode` for details.
 
 
 .. tabularcolumns:: |p{1.6cm}|p{3.2cm}|p{12.7cm}|
@@ -218,7 +218,15 @@ desired arrays with the media graph elements.
-  Pad flags, see :ref:`media-pad-flag` for more details.
 
 *  -  __u32
-   -  ``reserved``\ [5]
+   -  ``index``
+   -  Pad index, starts at 0. Only valid if 
``MEDIA_V2_PAD_HAS_INDEX(media_version)``
+ returns true. The ``media_version`` is defined in struct
+ :c:type:`media_device_info` and can be retrieved using
+ :ref:`MEDIA_IOC_DEVICE_INFO`. Pad indices are stable. If new pads are 
added
+ for an entity in the future, then those will be added at the end.
+
+*  -  __u32
+   -  ``reserved``\ [4]
-  Reserved for future extensions. Drivers and applications must set
  this array to zero.
 
-- 
2.18.0



[PATCHv6 10/12] media/i2c: add missing entity functions

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

Several drivers in media/i2c do not set the entity function.
Correct this.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/i2c/et8ek8/et8ek8_driver.c | 1 +
 drivers/media/i2c/mt9m032.c  | 1 +
 drivers/media/i2c/mt9p031.c  | 1 +
 drivers/media/i2c/mt9t001.c  | 1 +
 drivers/media/i2c/mt9v032.c  | 1 +
 5 files changed, 5 insertions(+)

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c 
b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index e9eff9039ef5..37ef38947e01 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -1446,6 +1446,7 @@ static int et8ek8_probe(struct i2c_client *client,
sensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
sensor->subdev.internal_ops = &et8ek8_internal_ops;
 
+   sensor->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(&sensor->subdev.entity, 1, &sensor->pad);
if (ret < 0) {
diff --git a/drivers/media/i2c/mt9m032.c b/drivers/media/i2c/mt9m032.c
index 6a9e068462fd..b385f2b632ad 100644
--- a/drivers/media/i2c/mt9m032.c
+++ b/drivers/media/i2c/mt9m032.c
@@ -793,6 +793,7 @@ static int mt9m032_probe(struct i2c_client *client,
v4l2_ctrl_cluster(2, &sensor->hflip);
 
sensor->subdev.ctrl_handler = &sensor->ctrls;
+   sensor->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(&sensor->subdev.entity, 1, &sensor->pad);
if (ret < 0)
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 91d822fc4443..715be3632b01 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -,6 +,7 @@ static int mt9p031_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(&mt9p031->subdev, client, &mt9p031_subdev_ops);
mt9p031->subdev.internal_ops = &mt9p031_subdev_internal_ops;
 
+   mt9p031->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
mt9p031->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(&mt9p031->subdev.entity, 1, &mt9p031->pad);
if (ret < 0)
diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
index 9d981d9f5686..f683d2cb0486 100644
--- a/drivers/media/i2c/mt9t001.c
+++ b/drivers/media/i2c/mt9t001.c
@@ -943,6 +943,7 @@ static int mt9t001_probe(struct i2c_client *client,
mt9t001->subdev.internal_ops = &mt9t001_subdev_internal_ops;
mt9t001->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
+   mt9t001->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
mt9t001->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(&mt9t001->subdev.entity, 1, &mt9t001->pad);
 
diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c
index 4de63b2df334..f74730d24d8f 100644
--- a/drivers/media/i2c/mt9v032.c
+++ b/drivers/media/i2c/mt9v032.c
@@ -1162,6 +1162,7 @@ static int mt9v032_probe(struct i2c_client *client,
mt9v032->subdev.internal_ops = &mt9v032_subdev_internal_ops;
mt9v032->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
+   mt9v032->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
mt9v032->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(&mt9v032->subdev.entity, 1, &mt9v032->pad);
if (ret < 0)
-- 
2.18.0



[PATCHv6 00/12] media/mc: fix inconsistencies

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

This series is v6 of my previous attempt:

https://www.mail-archive.com/linux-media@vger.kernel.org/msg133178.html

The goal is to fix the inconsistencies between the 'old' and 'new' 
MC API. I hate the terms 'old' and 'new', there is nothing wrong IMHO with 
using an 'old' API if it meets the needs of the application.

Making G_TOPOLOGY useful is urgently needed since I think that will be
very helpful for the work we want to do on library support for complex
cameras.

Changes since v5:

- Added Reviewed-by tags from Laurent.
- Improved the commit log of patch 05/12 mentioning that it also modifies
  drivers.
- Rephrased the pad index explanation in patch 11/12: new pad indices do
  not necessarily have to be added to the end. The key is that existing pad
  indices should never be renumbered. Since there can be holes in the pad
  indices it is always possible that a new pad uses one of those unused
  indices.

Changes since v4:

- Improve documentation of the new index field in patch 2.
- Added patch 11 to sync the index field documentation in 
media-ioc-enum-links.rst
  with the index field documentation from media-ioc-g-topology.rst.
- Added patch 12 that clarifies that you should not hardcode ID values
  in applications since they can change between instances of the device.
  Also document that the entity name is unique within the media topology.

Changes since v3:

- Renamed MEDIA_ENT_F_DTV_ENCODER to MEDIA_ENT_F_DV_ENCODER
- Added a new patch renaming MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER.
- Added a new patch that reorders the function defines in media.h so that they
  are in increasing numerical order (the en/decoder functions where not in
  order).
- Added Sakari's Acks (except for the two new patches).

Regards,

Hans


Hans Verkuil (12):
  media: add 'index' to struct media_v2_pad
  media-ioc-g-topology.rst: document new 'index' field
  media: add flags field to struct media_v2_entity
  media-ioc-g-topology.rst: document new 'flags' field
  media: rename MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER
  media.h: add MEDIA_ENT_F_DV_ENCODER
  media.h: reorder video en/decoder functions
  ad9389b/adv7511: set proper media entity function
  adv7180/tvp514x/tvp7002: fix entity function
  media/i2c: add missing entity functions
  media-ioc-enum-links.rst: improve pad index description
  media-ioc-enum-entities.rst/-g-topology.rst: clarify ID/name usage

 .../uapi/mediactl/media-ioc-enum-entities.rst |  9 ++--
 .../uapi/mediactl/media-ioc-enum-links.rst|  4 +-
 .../uapi/mediactl/media-ioc-g-topology.rst| 42 +++
 .../media/uapi/mediactl/media-types.rst   |  9 +++-
 drivers/media/i2c/ad9389b.c   |  1 +
 drivers/media/i2c/adv7180.c   |  2 +-
 drivers/media/i2c/adv7511.c   |  1 +
 drivers/media/i2c/adv7604.c   |  1 +
 drivers/media/i2c/adv7842.c   |  1 +
 drivers/media/i2c/et8ek8/et8ek8_driver.c  |  1 +
 drivers/media/i2c/mt9m032.c   |  1 +
 drivers/media/i2c/mt9p031.c   |  1 +
 drivers/media/i2c/mt9t001.c   |  1 +
 drivers/media/i2c/mt9v032.c   |  1 +
 drivers/media/i2c/tda1997x.c  |  2 +-
 drivers/media/i2c/tvp514x.c   |  2 +-
 drivers/media/i2c/tvp7002.c   |  2 +-
 drivers/media/media-device.c  |  2 +
 include/uapi/linux/media.h| 39 +
 19 files changed, 97 insertions(+), 25 deletions(-)

-- 
2.18.0



[PATCHv6 08/12] ad9389b/adv7511: set proper media entity function

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

These two drivers both have function MEDIA_ENT_F_DV_ENCODER.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/i2c/ad9389b.c | 1 +
 drivers/media/i2c/adv7511.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c
index 91ff06088572..5b008b0002c0 100644
--- a/drivers/media/i2c/ad9389b.c
+++ b/drivers/media/i2c/ad9389b.c
@@ -1134,6 +1134,7 @@ static int ad9389b_probe(struct i2c_client *client, const 
struct i2c_device_id *
goto err_hdl;
}
state->pad.flags = MEDIA_PAD_FL_SINK;
+   sd->entity.function = MEDIA_ENT_F_DV_ENCODER;
err = media_entity_pads_init(&sd->entity, 1, &state->pad);
if (err)
goto err_hdl;
diff --git a/drivers/media/i2c/adv7511.c b/drivers/media/i2c/adv7511.c
index 5731751d3f2a..55c2ea0720d9 100644
--- a/drivers/media/i2c/adv7511.c
+++ b/drivers/media/i2c/adv7511.c
@@ -1847,6 +1847,7 @@ static int adv7511_probe(struct i2c_client *client, const 
struct i2c_device_id *
goto err_hdl;
}
state->pad.flags = MEDIA_PAD_FL_SINK;
+   sd->entity.function = MEDIA_ENT_F_DV_ENCODER;
err = media_entity_pads_init(&sd->entity, 1, &state->pad);
if (err)
goto err_hdl;
-- 
2.18.0



[PATCHv6 05/12] media: rename MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

The use of 'DTV' is very confusing since it normally refers to Digital
TV e.g. DVB etc.

Instead use 'DV' (Digital Video), which nicely corresponds to the
DV Timings API used to configure such receivers and transmitters.

We keep an alias to avoid breaking userspace applications.

Since this alias is only available if __KERNEL__ is *not* defined
(i.e. it is only available for userspace, not kernelspace), any
drivers that use it also have to be converted to the new define.
These drivers are adv7604, adv7842 and tda1997x.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/mediactl/media-types.rst | 2 +-
 drivers/media/i2c/adv7604.c   | 1 +
 drivers/media/i2c/adv7842.c   | 1 +
 drivers/media/i2c/tda1997x.c  | 2 +-
 include/uapi/linux/media.h| 4 +++-
 5 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-types.rst 
b/Documentation/media/uapi/mediactl/media-types.rst
index 96910cf2eaaa..c11b0c7e890b 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -200,7 +200,7 @@ Types and flags used to represent the media graph elements
  MIPI CSI-2, etc.), and outputs them on its source pad to an output
  video bus of another type (eDP, MIPI CSI-2, parallel, etc.).
 
-*  -  ``MEDIA_ENT_F_DTV_DECODER``
+*  -  ``MEDIA_ENT_F_DV_DECODER``
-  Digital video decoder. The basic function of the video decoder is
  to accept digital video from a wide variety of sources
  and output it in some digital video standard, with appropriate
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 1a3b2c04d9f9..668be2bca57a 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -3499,6 +3499,7 @@ static int adv76xx_probe(struct i2c_client *client,
for (i = 0; i < state->source_pad; ++i)
state->pads[i].flags = MEDIA_PAD_FL_SINK;
state->pads[state->source_pad].flags = MEDIA_PAD_FL_SOURCE;
+   sd->entity.function = MEDIA_ENT_F_DV_DECODER;
 
err = media_entity_pads_init(&sd->entity, state->source_pad + 1,
state->pads);
diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c
index fddac32e5051..99d781343fb1 100644
--- a/drivers/media/i2c/adv7842.c
+++ b/drivers/media/i2c/adv7842.c
@@ -3541,6 +3541,7 @@ static int adv7842_probe(struct i2c_client *client,
INIT_DELAYED_WORK(&state->delayed_work_enable_hotplug,
adv7842_delayed_work_enable_hotplug);
 
+   sd->entity.function = MEDIA_ENT_F_DV_DECODER;
state->pad.flags = MEDIA_PAD_FL_SOURCE;
err = media_entity_pads_init(&sd->entity, 1, &state->pad);
if (err)
diff --git a/drivers/media/i2c/tda1997x.c b/drivers/media/i2c/tda1997x.c
index 039a92c3294a..d114ac5243ec 100644
--- a/drivers/media/i2c/tda1997x.c
+++ b/drivers/media/i2c/tda1997x.c
@@ -2570,7 +2570,7 @@ static int tda1997x_probe(struct i2c_client *client,
 id->name, i2c_adapter_id(client->adapter),
 client->addr);
sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
-   sd->entity.function = MEDIA_ENT_F_DTV_DECODER;
+   sd->entity.function = MEDIA_ENT_F_DV_DECODER;
sd->entity.ops = &tda1997x_media_ops;
 
/* set allowed mbus modes based on chip, bus-type, and bus-width */
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index ebd2cda67833..99f5e0978ebb 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -93,7 +93,7 @@ struct media_device_info {
  * Video decoder functions
  */
 #define MEDIA_ENT_F_ATV_DECODER
(MEDIA_ENT_F_OLD_SUBDEV_BASE + 4)
-#define MEDIA_ENT_F_DTV_DECODER(MEDIA_ENT_F_BASE + 
0x6001)
+#define MEDIA_ENT_F_DV_DECODER (MEDIA_ENT_F_BASE + 0x6001)
 
 /*
  * Digital TV, analog TV, radio and/or software defined radio tuner functions.
@@ -400,6 +400,8 @@ struct media_v2_topology {
 #define MEDIA_ENT_T_V4L2_SUBDEV_DECODERMEDIA_ENT_F_ATV_DECODER
 #define MEDIA_ENT_T_V4L2_SUBDEV_TUNER  MEDIA_ENT_F_TUNER
 
+#define MEDIA_ENT_F_DTV_DECODERMEDIA_ENT_F_DV_DECODER
+
 /*
  * There is still no ALSA support in the media controller. These
  * defines should not have been added and we leave them here only
-- 
2.18.0



[PATCHv6 12/12] media-ioc-enum-entities.rst/-g-topology.rst: clarify ID/name usage

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

Mention that IDs should not be hardcoded in applications and that the
entity name must be unique within the media topology.

Signed-off-by: Hans Verkuil 
Reviewed-by: Laurent Pinchart 
---
 .../uapi/mediactl/media-ioc-enum-entities.rst |  9 +---
 .../uapi/mediactl/media-ioc-g-topology.rst| 22 ++-
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst 
b/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
index 961466ae821d..fc2e39c070c9 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst
@@ -62,15 +62,18 @@ id's until they get an error.
-  ``id``
-
-
-   -  Entity id, set by the application. When the id is or'ed with
+   -  Entity ID, set by the application. When the ID is or'ed with
  ``MEDIA_ENT_ID_FLAG_NEXT``, the driver clears the flag and returns
- the first entity with a larger id.
+ the first entity with a larger ID. Do not expect that the ID will
+ always be the same for each instance of the device. In other words,
+ do not hardcode entity IDs in an application.
 
 *  -  char
-  ``name``\ [32]
-
-
-   -  Entity name as an UTF-8 NULL-terminated string.
+   -  Entity name as an UTF-8 NULL-terminated string. This name must be 
unique
+  within the media topology.
 
 *  -  __u32
-  ``type``
diff --git a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst 
b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
index e572dd0d806d..3a5f165d9811 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-g-topology.rst
@@ -131,11 +131,14 @@ desired arrays with the media graph elements.
 
 *  -  __u32
-  ``id``
-   -  Unique ID for the entity.
+   -  Unique ID for the entity. Do not expect that the ID will
+ always be the same for each instance of the device. In other words,
+ do not hardcode entity IDs in an application.
 
 *  -  char
-  ``name``\ [64]
-   -  Entity name as an UTF-8 NULL-terminated string.
+   -  Entity name as an UTF-8 NULL-terminated string. This name must be 
unique
+  within the media topology.
 
 *  -  __u32
-  ``function``
@@ -166,7 +169,9 @@ desired arrays with the media graph elements.
 
 *  -  __u32
-  ``id``
-   -  Unique ID for the interface.
+   -  Unique ID for the interface. Do not expect that the ID will
+ always be the same for each instance of the device. In other words,
+ do not hardcode interface IDs in an application.
 
 *  -  __u32
-  ``intf_type``
@@ -215,7 +220,9 @@ desired arrays with the media graph elements.
 
 *  -  __u32
-  ``id``
-   -  Unique ID for the pad.
+   -  Unique ID for the pad. Do not expect that the ID will
+ always be the same for each instance of the device. In other words,
+ do not hardcode pad IDs in an application.
 
 *  -  __u32
-  ``entity_id``
@@ -231,7 +238,8 @@ desired arrays with the media graph elements.
  returns true. The ``media_version`` is defined in struct
  :c:type:`media_device_info` and can be retrieved using
  :ref:`MEDIA_IOC_DEVICE_INFO`. Pad indices are stable. If new pads are 
added
- for an entity in the future, then those will be added at the end.
+ for an entity in the future, then those will be added at the end of 
the
+ entity's pad array.
 
 *  -  __u32
-  ``reserved``\ [4]
@@ -250,7 +258,9 @@ desired arrays with the media graph elements.
 
 *  -  __u32
-  ``id``
-   -  Unique ID for the link.
+   -  Unique ID for the link. Do not expect that the ID will
+ always be the same for each instance of the device. In other words,
+ do not hardcode link IDs in an application.
 
 *  -  __u32
-  ``source_id``
-- 
2.18.0



[PATCHv6 11/12] media-ioc-enum-links.rst: improve pad index description

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

Make it clearer that the index starts at 0, and that it won't change
since future new pads will never renumber existing pad indices.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/mediactl/media-ioc-enum-links.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst 
b/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
index 17abdeed1a9c..f5cd75b42b69 100644
--- a/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
+++ b/Documentation/media/uapi/mediactl/media-ioc-enum-links.rst
@@ -92,7 +92,9 @@ returned during the enumeration process.
 
 *  -  __u16
-  ``index``
-   -  0-based pad index.
+   -  Pad index, starts at 0. Pad indices are stable. So if new pads are 
added
+ for an entity in the future, then the indices of the existing pads 
will
+ never be renumbered, they will remain the same.
 
 *  -  __u32
-  ``flags``
-- 
2.18.0



[PATCHv6 03/12] media: add flags field to struct media_v2_entity

2018-07-10 Thread Hans Verkuil
From: Hans Verkuil 

The v2 entity structure never exposed the entity flags, which made it
impossible to detect connector or default entities.

It is really trivial to just expose this information, so implement this.

Signed-off-by: Hans Verkuil 
Acked-by: Sakari Ailus 
---
 drivers/media/media-device.c |  1 +
 include/uapi/linux/media.h   | 12 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 047d38372a27..14959b19a342 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -266,6 +266,7 @@ static long media_device_get_topology(struct media_device 
*mdev, void *arg)
memset(&kentity, 0, sizeof(kentity));
kentity.id = entity->graph_obj.id;
kentity.function = entity->function;
+   kentity.flags = entity->flags;
strlcpy(kentity.name, entity->name,
sizeof(kentity.name));
 
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index f6338bd57929..ebd2cda67833 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -280,11 +280,21 @@ struct media_links_enum {
  * MC next gen API definitions
  */
 
+/*
+ * Appeared in 4.19.0.
+ *
+ * The media_version argument comes from the media_version field in
+ * struct media_device_info.
+ */
+#define MEDIA_V2_ENTITY_HAS_FLAGS(media_version) \
+   ((media_version) >= ((4 << 16) | (19 << 8) | 0))
+
 struct media_v2_entity {
__u32 id;
char name[64];
__u32 function; /* Main function of the entity */
-   __u32 reserved[6];
+   __u32 flags;
+   __u32 reserved[5];
 } __attribute__ ((packed));
 
 /* Should match the specific fields at media_intf_devnode */
-- 
2.18.0



[GIT PULL FOR v4.19] media/mc: fix inconsistencies

2018-07-10 Thread Hans Verkuil
This is the pull request of v6 of my patch series to fix the mc inconsistencies.

Regards,

Hans

The following changes since commit 666e994aa2278e948e2492ee9d81b4df241e7222:

  media: platform: s5p-mfc: simplify getting .drvdata (2018-07-04 11:45:40 
-0400)

are available in the Git repository at:

  git://linuxtv.org/hverkuil/media_tree.git media-missing2

for you to fetch changes up to 607488f06efa4e87be5eb229759105d990f7df18:

  media-ioc-enum-entities.rst/-g-topology.rst: clarify ID/name usage 
(2018-07-10 10:40:19 +0200)


Hans Verkuil (12):
  media: add 'index' to struct media_v2_pad
  media-ioc-g-topology.rst: document new 'index' field
  media: add flags field to struct media_v2_entity
  media-ioc-g-topology.rst: document new 'flags' field
  media: rename MEDIA_ENT_F_DTV_DECODER to MEDIA_ENT_F_DV_DECODER
  media.h: add MEDIA_ENT_F_DV_ENCODER
  media.h: reorder video en/decoder functions
  ad9389b/adv7511: set proper media entity function
  adv7180/tvp514x/tvp7002: fix entity function
  media/i2c: add missing entity functions
  media-ioc-enum-links.rst: improve pad index description
  media-ioc-enum-entities.rst/-g-topology.rst: clarify ID/name usage

 Documentation/media/uapi/mediactl/media-ioc-enum-entities.rst |  9 ++---
 Documentation/media/uapi/mediactl/media-ioc-enum-links.rst|  4 +++-
 Documentation/media/uapi/mediactl/media-ioc-g-topology.rst| 42 
++
 Documentation/media/uapi/mediactl/media-types.rst |  9 -
 drivers/media/i2c/ad9389b.c   |  1 +
 drivers/media/i2c/adv7180.c   |  2 +-
 drivers/media/i2c/adv7511.c   |  1 +
 drivers/media/i2c/adv7604.c   |  1 +
 drivers/media/i2c/adv7842.c   |  1 +
 drivers/media/i2c/et8ek8/et8ek8_driver.c  |  1 +
 drivers/media/i2c/mt9m032.c   |  1 +
 drivers/media/i2c/mt9p031.c   |  1 +
 drivers/media/i2c/mt9t001.c   |  1 +
 drivers/media/i2c/mt9v032.c   |  1 +
 drivers/media/i2c/tda1997x.c  |  2 +-
 drivers/media/i2c/tvp514x.c   |  2 +-
 drivers/media/i2c/tvp7002.c   |  2 +-
 drivers/media/media-device.c  |  2 ++
 include/uapi/linux/media.h| 39 
+++
 19 files changed, 97 insertions(+), 25 deletions(-)


hi

2018-07-10 Thread Sgt Sherri Gallagher
Please reply me back.I am Sgt.Sherri


Re: [PATCH -next v3 1/2] i2c: add SCCB helpers

2018-07-10 Thread Laurent Pinchart
Hi Akinobu,

Thank you for the patch.

On Monday, 9 July 2018 18:41:13 EEST Akinobu Mita wrote:
> This adds Serial Camera Control Bus (SCCB) helpers (sccb_is_available,
> sccb_read_byte, and sccb_write_byte) that are intended to be used by some
> of Omnivision sensor drivers.
> 
> The ov772x driver is going to use these helpers.
> 
> It was previously only worked with the i2c controller drivers that support
> I2C_FUNC_PROTOCOL_MANGLING, because the ov772x device doesn't support
> repeated starts.  After commit 0b964d183cbf ("media: ov772x: allow i2c
> controllers without I2C_FUNC_PROTOCOL_MANGLING"), reading ov772x register
> is replaced with issuing two separated i2c messages in order to avoid
> repeated start.  Using SCCB helpers hides the implementation detail.
> 
> Cc: Peter Rosin 
> Cc: Sebastian Reichel 
> Cc: Wolfram Sang 
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Akinobu Mita 
> ---
>  include/linux/i2c-sccb.h | 77 +
>  1 file changed, 77 insertions(+)
>  create mode 100644 include/linux/i2c-sccb.h
> 
> diff --git a/include/linux/i2c-sccb.h b/include/linux/i2c-sccb.h
> new file mode 100644
> index 000..61dece9
> --- /dev/null
> +++ b/include/linux/i2c-sccb.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Serial Camera Control Bus (SCCB) helper functions
> + */
> +
> +#ifndef _LINUX_I2C_SCCB_H
> +#define _LINUX_I2C_SCCB_H
> +
> +#include 
> +
> +/**
> + * sccb_is_available - Check if the adapter supports SCCB protocol
> + * @adap: I2C adapter
> + *
> + * Return true if the I2C adapter is capable of using SCCB helper
> functions, + * false otherwise.
> + */
> +static inline bool sccb_is_available(struct i2c_adapter *adap)
> +{
> + u32 needed_funcs = I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA;
> +
> +#if 0
> + /*
> +  * sccb_xfer not needed yet, since there is no driver support currently.
> +  * Just showing how it should be done if we ever need it.
> +  */
> + if (adap->algo->sccb_xfer)
> + return true;
> +#endif
> +
> + return (i2c_get_functionality(adap) & needed_funcs) == needed_funcs;
> +}
> +
> +/**
> + * sccb_read_byte - Read data from SCCB slave device
> + * @client: Handle to slave device
> + * @addr: Register to be read from
> + *
> + * This executes the 2-phase write transmission cycle that is followed by a
> + * 2-phase read transmission cycle, returning negative errno else a data
> byte
> + * received from the device.
> + */
> +static inline int sccb_read_byte(struct i2c_client *client, u8 addr)
> +{
> + int ret;
> + union i2c_smbus_data data;
> +
> + i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
> +
> + ret = __i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> + I2C_SMBUS_WRITE, addr, I2C_SMBUS_BYTE, NULL);
> + if (ret < 0)
> + goto out;
> +
> + ret = __i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> + I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &data);
> +out:
> + i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
> +
> + return ret < 0 ? ret : data.byte;
> +}

I think I mentioned in a previous review of this patch that the function is 
too big to be a static inline. It should instead be moved to a .c file.

> +/**
> + * sccb_write_byte - Write data to SCCB slave device
> + * @client: Handle to slave device
> + * @addr: Register to write to
> + * @data: Value to be written
> + *
> + * This executes the SCCB 3-phase write transmission cycle, returning
> negative
> + * errno else zero on success.
> + */
> +static inline int sccb_write_byte(struct i2c_client *client, u8 addr, u8
> data)
> +{
> + return i2c_smbus_write_byte_data(client, addr, data);
> +}
> +
> +#endif /* _LINUX_I2C_SCCB_H */

-- 
Regards,

Laurent Pinchart





Re: [PATCH] media: imx208: Add imx208 camera sensor driver

2018-07-10 Thread kbuild test robot
Hi Ping-chung,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.18-rc4 next-20180709]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Ping-chung-Chen/media-imx208-Add-imx208-camera-sensor-driver/20180710-153546
base:   git://linuxtv.org/media_tree.git master
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/media/i2c/imx208.c:881:26: sparse: no member 'type' in struct 
media_entity
   drivers/media/i2c/imx208.c:885:15: sparse: undefined identifier 
'media_entity_init'
   drivers/media/i2c/imx208.c:881:26: sparse: generating address of non-lvalue 
(8)
   drivers/media/i2c/imx208.c:885:32: sparse: call with no type!
   drivers/media/i2c/imx208.c: In function 'imx208_probe':
>> drivers/media/i2c/imx208.c:881:20: error: 'struct media_entity' has no 
>> member named 'type'; did you mean 'pipe'?
 imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
   ^~~~
   pipe
>> drivers/media/i2c/imx208.c:881:27: error: 'MEDIA_ENT_T_V4L2_SUBDEV_SENSOR' 
>> undeclared (first use in this function); did you mean 
>> 'MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN'?
 imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
  ^~
  MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN
   drivers/media/i2c/imx208.c:881:27: note: each undeclared identifier is 
reported only once for each function it appears in
>> drivers/media/i2c/imx208.c:885:8: error: implicit declaration of function 
>> 'media_entity_init'; did you mean 'media_entity_put'? 
>> [-Werror=implicit-function-declaration]
 ret = media_entity_init(&imx208->sd.entity, 1, &imx208->pad, 0);
   ^
   media_entity_put
   cc1: some warnings being treated as errors

sparse warnings: (new ones prefixed by >>)

   drivers/media/i2c/imx208.c:881:26: sparse: no member 'type' in struct 
media_entity
   drivers/media/i2c/imx208.c:885:15: sparse: undefined identifier 
'media_entity_init'
>> drivers/media/i2c/imx208.c:881:26: sparse: generating address of non-lvalue 
>> (8)
>> drivers/media/i2c/imx208.c:885:32: sparse: call with no type!
   drivers/media/i2c/imx208.c: In function 'imx208_probe':
   drivers/media/i2c/imx208.c:881:20: error: 'struct media_entity' has no 
member named 'type'; did you mean 'pipe'?
 imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
   ^~~~
   pipe
   drivers/media/i2c/imx208.c:881:27: error: 'MEDIA_ENT_T_V4L2_SUBDEV_SENSOR' 
undeclared (first use in this function); did you mean 
'MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN'?
 imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
  ^~
  MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN
   drivers/media/i2c/imx208.c:881:27: note: each undeclared identifier is 
reported only once for each function it appears in
   drivers/media/i2c/imx208.c:885:8: error: implicit declaration of function 
'media_entity_init'; did you mean 'media_entity_put'? 
[-Werror=implicit-function-declaration]
 ret = media_entity_init(&imx208->sd.entity, 1, &imx208->pad, 0);
   ^
   media_entity_put
   cc1: some warnings being treated as errors

vim +881 drivers/media/i2c/imx208.c

   848  
   849  static int imx208_probe(struct i2c_client *client)
   850  {
   851  struct imx208 *imx208;
   852  int ret;
   853  u32 val = 0;
   854  
   855  device_property_read_u32(&client->dev, "clock-frequency", &val);
   856  if (val != 1920)
   857  return -EINVAL;
   858  
   859  imx208 = devm_kzalloc(&client->dev, sizeof(*imx208), 
GFP_KERNEL);
   860  if (!imx208)
   861  return -ENOMEM;
   862  
   863  /* Initialize subdev */
   864  v4l2_i2c_subdev_init(&imx208->sd, client, &imx208_subdev_ops);
   865  
   866  /* Check module identity */
   867  ret = imx208_identify_module(imx208);
   868  if (ret)
   869  return ret;
   870  
   871  /* Set default mode to max resolution */
   872  imx208->cur_mode = &supported_modes[0];
   873  
   874  ret = imx208_init_controls(imx20

Re: [PATCH 16/16] media: imx: add mem2mem device

2018-07-10 Thread Pavel Machek
Hi!

> Add a single imx-media mem2mem video device that uses the IPU IC PP
> (image converter post processing) task for scaling and colorspace
> conversion.
> On i.MX6Q/DL SoCs with two IPUs currently only the first IPU is used.
> 
> The hardware only supports writing to destination buffers up to
> 1024x1024 pixels in a single pass, so the mem2mem video device is
> limited to this resolution. After fixing the tiling code it should
> be possible to extend this to arbitrary sizes by rendering multiple
> tiles per frame.
> 
> Signed-off-by: Philipp Zabel 

> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * i.MX IPUv3 mem2mem Scaler/CSC driver
> + *
> + * Copyright (C) 2011 Pengutronix, Sascha Hauer
> + * Copyright (C) 2018 Pengutronix, Philipp Zabel
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */

Point of SPDX is that the last 4 lines can be removed...and if you
want GPL-2.0+ as you state (and I like that), you should also say so
in SPDX.

Thanks,

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


Re: [PATCH -next v3 1/2] i2c: add SCCB helpers

2018-07-10 Thread Wolfram Sang

> > +static inline int sccb_read_byte(struct i2c_client *client, u8 addr)
> > +{
> > +   int ret;
> > +   union i2c_smbus_data data;
> > +
> > +   i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
> > +
> > +   ret = __i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> > +   I2C_SMBUS_WRITE, addr, I2C_SMBUS_BYTE, NULL);
> > +   if (ret < 0)
> > +   goto out;
> > +
> > +   ret = __i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> > +   I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &data);
> > +out:
> > +   i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
> > +
> > +   return ret < 0 ? ret : data.byte;
> > +}
> 
> I think I mentioned in a previous review of this patch that the function is 
> too big to be a static inline. It should instead be moved to a .c file.

Can be argued. I assume just removing the 'inline' won't do it for you?
I'd be fine with that, there are not many SCCB useres out there...

But if you insist on drivers/i2c/i2c-sccb.c, then it should be a
seperate module, I'd think?



signature.asc
Description: PGP signature


Re: [PATCH] media: imx208: Add imx208 camera sensor driver

2018-07-10 Thread kbuild test robot
Hi Ping-chung,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.18-rc4 next-20180709]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Ping-chung-Chen/media-imx208-Add-imx208-camera-sensor-driver/20180710-153546
base:   git://linuxtv.org/media_tree.git master
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/media/i2c/imx208.c: In function 'imx208_probe':
   drivers/media/i2c/imx208.c:881:20: error: 'struct media_entity' has no 
member named 'type'; did you mean 'pipe'?
 imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
   ^~~~
   pipe
>> drivers/media/i2c/imx208.c:881:27: error: 'MEDIA_ENT_T_V4L2_SUBDEV_SENSOR' 
>> undeclared (first use in this function); did you mean 
>> 'MEDIA_ENTITY_TYPE_V4L2_SUBDEV'?
 imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
  ^~
  MEDIA_ENTITY_TYPE_V4L2_SUBDEV
   drivers/media/i2c/imx208.c:881:27: note: each undeclared identifier is 
reported only once for each function it appears in
   drivers/media/i2c/imx208.c:885:8: error: implicit declaration of function 
'media_entity_init'; did you mean 'media_entity_put'? 
[-Werror=implicit-function-declaration]
 ret = media_entity_init(&imx208->sd.entity, 1, &imx208->pad, 0);
   ^
   media_entity_put
   cc1: some warnings being treated as errors

vim +881 drivers/media/i2c/imx208.c

   848  
   849  static int imx208_probe(struct i2c_client *client)
   850  {
   851  struct imx208 *imx208;
   852  int ret;
   853  u32 val = 0;
   854  
   855  device_property_read_u32(&client->dev, "clock-frequency", &val);
   856  if (val != 1920)
   857  return -EINVAL;
   858  
   859  imx208 = devm_kzalloc(&client->dev, sizeof(*imx208), 
GFP_KERNEL);
   860  if (!imx208)
   861  return -ENOMEM;
   862  
   863  /* Initialize subdev */
   864  v4l2_i2c_subdev_init(&imx208->sd, client, &imx208_subdev_ops);
   865  
   866  /* Check module identity */
   867  ret = imx208_identify_module(imx208);
   868  if (ret)
   869  return ret;
   870  
   871  /* Set default mode to max resolution */
   872  imx208->cur_mode = &supported_modes[0];
   873  
   874  ret = imx208_init_controls(imx208);
   875  if (ret)
   876  return ret;
   877  
   878  /* Initialize subdev */
   879  imx208->sd.internal_ops = &imx208_internal_ops;
   880  imx208->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 > 881  imx208->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
   882  
   883  /* Initialize source pad */
   884  imx208->pad.flags = MEDIA_PAD_FL_SOURCE;
   885  ret = media_entity_init(&imx208->sd.entity, 1, &imx208->pad, 0);
   886  if (ret) {
   887  dev_err(&client->dev, "%s failed:%d\n", __func__, ret);
   888  goto error_handler_free;
   889  }
   890  
   891  ret = v4l2_async_register_subdev_sensor_common(&imx208->sd);
   892  if (ret < 0)
   893  goto error_media_entity;
   894  
   895  pm_runtime_set_active(&client->dev);
   896  pm_runtime_enable(&client->dev);
   897  pm_runtime_idle(&client->dev);
   898  
   899  return 0;
   900  
   901  error_media_entity:
   902  media_entity_cleanup(&imx208->sd.entity);
   903  
   904  error_handler_free:
   905  imx208_free_controls(imx208);
   906  
   907  return ret;
   908  }
   909  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip


Re: [PATCH -next v3 1/2] i2c: add SCCB helpers

2018-07-10 Thread Laurent Pinchart
Hi Wolfram,

On Tuesday, 10 July 2018 15:07:47 EEST Wolfram Sang wrote:
> >> +static inline int sccb_read_byte(struct i2c_client *client, u8 addr)
> >> +{
> >> +  int ret;
> >> +  union i2c_smbus_data data;
> >> +
> >> +  i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
> >> +
> >> +  ret = __i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> >> +  I2C_SMBUS_WRITE, addr, I2C_SMBUS_BYTE, NULL);
> >> +  if (ret < 0)
> >> +  goto out;
> >> +
> >> +  ret = __i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> >> +  I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &data);
> >> +out:
> >> +  i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
> >> +
> >> +  return ret < 0 ? ret : data.byte;
> >> +}
> > 
> > I think I mentioned in a previous review of this patch that the function
> > is too big to be a static inline. It should instead be moved to a .c file.
> 
> Can be argued.

Especially if sccb_read_byte() is called in multiple places in a driver, not 
just once in a read helper, as you've advised for patch 2/2 in this series :-)

> I assume just removing the 'inline' won't do it for you?

Just removing the inline keyword will create many instances of the function, 
even when not used. I think it will also cause the compiler to emit warnings 
for unused functions. I don't think that's a good idea.

> I'd be fine with that, there are not many SCCB useres out there...
> 
> But if you insist on drivers/i2c/i2c-sccb.c, then it should be a
> seperate module, I'd think?

Given how small the functions are, I wouldn't request that, as it would 
introduce another Kconfig symbol, but I'm not opposed to such a new module 
either.

-- 
Regards,

Laurent Pinchart





Re: [PATCH -next v3 1/2] i2c: add SCCB helpers

2018-07-10 Thread Wolfram Sang

> even when not used. I think it will also cause the compiler to emit warnings 
> for unused functions. I don't think that's a good idea.

Where is my brown paper bag? :/

> > But if you insist on drivers/i2c/i2c-sccb.c, then it should be a
> > seperate module, I'd think?
> 
> Given how small the functions are, I wouldn't request that, as it would 
> introduce another Kconfig symbol, but I'm not opposed to such a new module 
> either.

OK, let's keep it simple for now and introduce
drivers/i2c/i2c-core-sccb.c and link it into the core module. It can
still be factored out later if the need arises.



signature.asc
Description: PGP signature


Re: [PATCH -next v3 2/2] media: ov772x: use SCCB helpers

2018-07-10 Thread Akinobu Mita
2018年7月10日(火) 6:23 Sebastian Reichel :
>
> Hi,
>
> On Mon, Jul 09, 2018 at 06:14:43PM +0200, Wolfram Sang wrote:
> > >  static int ov772x_read(struct i2c_client *client, u8 addr)
> > >  {
> > > -   int ret;
> > > -   u8 val;
> > > -
> > > -   ret = i2c_master_send(client, &addr, 1);
> > > -   if (ret < 0)
> > > -   return ret;
> > > -   ret = i2c_master_recv(client, &val, 1);
> > > -   if (ret < 0)
> > > -   return ret;
> > > -
> > > -   return val;
> > > +   return sccb_read_byte(client, addr);
> > >  }
> > >
> > >  static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 
> > > value)
> > >  {
> > > -   return i2c_smbus_write_byte_data(client, addr, value);
> > > +   return sccb_write_byte(client, addr, value);
> > >  }
>
> Reviewed-by: Sebastian Reichel 
>
> > Minor nit: I'd rather drop these two functions and use the
> > sccb-accessors directly.
> >
> > However, I really like how this looks here: It is totally clear we are
> > doing SCCB and hide away all the details.
>
> I think it would be even better to introduce a SSCB regmap layer
> and use that.

I'm fine with introducing a SCCB regmap layer.

But do we need to provide both a SCCB regmap and these SCCB helpers?

If we have a regmap_init_sccb(), I feel these three SCCB helper functions
don't need to be exported anymore.


Re: [PATCH] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-10 Thread Steve Longerbeam




On 07/10/2018 10:52 AM, jacopo mondi wrote:

Hi Steve,
I've done some more testing, and finally got MIPI capture work
properly on the i.MX6Q platform I'm testing on.

Apparently commit:
476dec012f4c6545b0b7599cd9adba2ed819ad3b
("media: ov5640: Add horizontal and vertical totals")
which several people, Jagan included, has reported introducing
regressions on MIPI actually breaks image capture.

I have a fix based on Maxime's and Sam's
[PATCH v3 01/12] media: ov5640: Fix timings setup code

I will send the two patches out soon.


Thanks J!

Steve




On Tue, Jul 10, 2018 at 09:33:31AM +0200, jacopo mondi wrote:

Hi Steve,
thanks for testing!

On Mon, Jul 09, 2018 at 02:52:09PM -0700, Steve Longerbeam wrote:

Hi Jacopo,

I tested this patch on the i.MX6Q SabreSD with the OV5640 module.
It fixes the LP-11 timeout at stream on, but the captured images
are completely blank/black.

Intersting that the module was not starting up properly on other
(all?) i.MX6Q platforms, not only on Engicam's one. I didn't get this
initially.

Ok, so that's a step forward, but not enough probably. I'll keep
looking into this and get back!

Thanks
j


Steve


On 07/06/2018 04:00 AM, Jacopo Mondi wrote:

From: Jacopo Mondi 

Rework the MIPI interface startup sequence with the following changes:

- Remove MIPI bus initialization from the initial settings blob
- At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in
   LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode.
- At s_stream time enable/disable the MIPI interface output.
- Restore default settings at set_power(0) time.

Before this commit the sensor MIPI interface was initialized with settings
that require a start/stop sequence at power-up time in order to force lanes
into LP11 state, as they were initialized in LP00 when in 'sleep mode',
which is assumed to be the sensor manual definition for the D-PHY defined
stop mode.

The stream start/stop was performed by enabling disabling clock gating,
and had the side effect to change the lanes sleep mode configuration when
stream was stopped.

Clock gating/ungating:
-   ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5),
-on ? 0 : BIT(5));
-   if (ret)

Set lanes in LP11 when in 'sleep mode':
-   ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
-  on ? 0x00 : 0x70);

This commit fixes an issue reported by Jagan Teki on i.MX6 platforms that
prevents the host interface from powering up correctly:
https://lkml.org/lkml/2018/6/1/38

It also improves MIPI capture operations stability on my testing platform
where MIPI capture often (silently) failed and returned all-purple frames.

fixes: f22996db44e2 ("media: ov5640: add support of DVP parallel interface")
Reported-by: Jagan Teki 
Signed-off-by: Jacopo Mondi 

---

Hello,
   I'm sending this one as new patch instead of a v2 of the previously sent
series "media: i2c: ov5640: Re-work MIPI interface configuration" as the
previous one was not working on the Engicam i.Mx6 platform where Jagan
initially reported issues on.

I've been able to test that capture now starts on said platform, but I've not
been able to visually verify any of the image content as I have no way yet to
transfer the raw images to my development host and verify their content (network
still not working for me on that platform :/ )

On my other testing platform images are correct, but they already were with the
previous version of this patches too, so I assume the CSI-2 receiver is far more
tolerant there.

Jagan, is there any way you could verify images? I would appreciate your
Tested-by tag in case they're correct.

Also, as there seems to be a lot of people interested in ov5640 these days, I
have expanded the receivers list. Anyone that could give these patches a spin?
(ie. Sam reported issues too with the current MIPI startup sequence in a patch
series he shared on dropbox iirc...)

Thanks
j
---
  drivers/media/i2c/ov5640.c | 91 --
  1 file changed, 71 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1ecbb7a..7bbd1d7 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -286,10 +286,10 @@ static const struct reg_value 
ov5640_init_setting_30fps_VGA[] = {
{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x3000, 0x00, 0, 0},
{0x3002, 0x1c, 0, 0}, {0x3004, 0xff, 0, 0}, {0x3006, 0xc3, 0, 0},
-   {0x300e, 0x45, 0, 0}, {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
+   {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
{0x501f, 0x00, 0, 0}, {0x4713, 0x03, 0, 0}, {0x4407, 0x04, 0, 0},
{0x440e, 0x00, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-   {0x4837, 0x0a, 0, 0}, {0x4800, 0x04, 0, 0}, {0x3824, 0x02, 0, 0},
+   {0x4837, 0x0a, 0, 0}, {0x3824, 0x02, 0,

[PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-10 Thread Jacopo Mondi
Hello,
   this series fixes capture operations on i.MX6Q platforms (and possible other
platforms reported not working) using MIPI CSI-2 interface.

This iteration expands the v1 version with an additional fix, initially
submitted by Maxime in his series:
[PATCH v3 00/12] media: ov5640: Misc cleanup and improvements
https://www.spinics.net/lists/linux-media/msg134436.html

The original patch has been reported not fully fixing the issues by Daniel Mack
in his comment here below (on a Qualcomm platform if I'm not wrong):
https://www.spinics.net/lists/linux-media/msg134524.html
On my i.MX6Q testing platform that patch alone does not fix MIPI capture
neither.

The version I'm sending here re-introduces some of the timings parameters in the
initial configuration blob (not in the single mode ones), which apparently has
to be at least initially programmed to allow the driver to later program them
singularly in the 'set_timings()' function. Unfortunately I do not have a real
rationale behind this which explains why it has to be done this way :(

For the MIPI startup sequence re-work patch, no changes compared to v1.
Steve reported he has verified the LP-11 timout issue is solved on his testing
platform too. For more details, please refer to the v1 cover letter:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg133352.html

Thanks
   j

Jacopo Mondi (1):
  media: i2c: ov5640: Re-work MIPI startup sequence

Samuel Bobrowicz (1):
  media: ov5640: Fix timings setup code

 drivers/media/i2c/ov5640.c | 107 ++---
 1 file changed, 82 insertions(+), 25 deletions(-)

--
2.7.4



Re: [PATCH] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-10 Thread jacopo mondi
Hi Steve,
   I've done some more testing, and finally got MIPI capture work
properly on the i.MX6Q platform I'm testing on.

Apparently commit:
476dec012f4c6545b0b7599cd9adba2ed819ad3b
("media: ov5640: Add horizontal and vertical totals")
which several people, Jagan included, has reported introducing
regressions on MIPI actually breaks image capture.

I have a fix based on Maxime's and Sam's
[PATCH v3 01/12] media: ov5640: Fix timings setup code

I will send the two patches out soon.

If you and other people are eager to test them it would be awesome to
have a confirmation MIPI capture works for you too.

Thanks
   j

On Tue, Jul 10, 2018 at 09:33:31AM +0200, jacopo mondi wrote:
> Hi Steve,
>thanks for testing!
>
> On Mon, Jul 09, 2018 at 02:52:09PM -0700, Steve Longerbeam wrote:
> > Hi Jacopo,
> >
> > I tested this patch on the i.MX6Q SabreSD with the OV5640 module.
> > It fixes the LP-11 timeout at stream on, but the captured images
> > are completely blank/black.
>
> Intersting that the module was not starting up properly on other
> (all?) i.MX6Q platforms, not only on Engicam's one. I didn't get this
> initially.
>
> Ok, so that's a step forward, but not enough probably. I'll keep
> looking into this and get back!
>
> Thanks
>j
>
> >
> > Steve
> >
> >
> > On 07/06/2018 04:00 AM, Jacopo Mondi wrote:
> > >From: Jacopo Mondi 
> > >
> > >Rework the MIPI interface startup sequence with the following changes:
> > >
> > >- Remove MIPI bus initialization from the initial settings blob
> > >- At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in
> > >   LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode.
> > >- At s_stream time enable/disable the MIPI interface output.
> > >- Restore default settings at set_power(0) time.
> > >
> > >Before this commit the sensor MIPI interface was initialized with settings
> > >that require a start/stop sequence at power-up time in order to force lanes
> > >into LP11 state, as they were initialized in LP00 when in 'sleep mode',
> > >which is assumed to be the sensor manual definition for the D-PHY defined
> > >stop mode.
> > >
> > >The stream start/stop was performed by enabling disabling clock gating,
> > >and had the side effect to change the lanes sleep mode configuration when
> > >stream was stopped.
> > >
> > >Clock gating/ungating:
> > >-   ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5),
> > >-on ? 0 : BIT(5));
> > >-   if (ret)
> > >
> > >Set lanes in LP11 when in 'sleep mode':
> > >-   ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
> > >-  on ? 0x00 : 0x70);
> > >
> > >This commit fixes an issue reported by Jagan Teki on i.MX6 platforms that
> > >prevents the host interface from powering up correctly:
> > >https://lkml.org/lkml/2018/6/1/38
> > >
> > >It also improves MIPI capture operations stability on my testing platform
> > >where MIPI capture often (silently) failed and returned all-purple frames.
> > >
> > >fixes: f22996db44e2 ("media: ov5640: add support of DVP parallel 
> > >interface")
> > >Reported-by: Jagan Teki 
> > >Signed-off-by: Jacopo Mondi 
> > >
> > >---
> > >
> > >Hello,
> > >   I'm sending this one as new patch instead of a v2 of the previously sent
> > >series "media: i2c: ov5640: Re-work MIPI interface configuration" as the
> > >previous one was not working on the Engicam i.Mx6 platform where Jagan
> > >initially reported issues on.
> > >
> > >I've been able to test that capture now starts on said platform, but I've 
> > >not
> > >been able to visually verify any of the image content as I have no way yet 
> > >to
> > >transfer the raw images to my development host and verify their content 
> > >(network
> > >still not working for me on that platform :/ )
> > >
> > >On my other testing platform images are correct, but they already were 
> > >with the
> > >previous version of this patches too, so I assume the CSI-2 receiver is 
> > >far more
> > >tolerant there.
> > >
> > >Jagan, is there any way you could verify images? I would appreciate your
> > >Tested-by tag in case they're correct.
> > >
> > >Also, as there seems to be a lot of people interested in ov5640 these 
> > >days, I
> > >have expanded the receivers list. Anyone that could give these patches a 
> > >spin?
> > >(ie. Sam reported issues too with the current MIPI startup sequence in a 
> > >patch
> > >series he shared on dropbox iirc...)
> > >
> > >Thanks
> > >j
> > >---
> > >  drivers/media/i2c/ov5640.c | 91 
> > > --
> > >  1 file changed, 71 insertions(+), 20 deletions(-)
> > >
> > >diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > >index 1ecbb7a..7bbd1d7 100644
> > >--- a/drivers/media/i2c/ov5640.c
> > >+++ b/drivers/media/i2c/ov5640.c
> > >@@ -286,10 +286,10 @@ static const struct reg_value 
> > >ov5640_init_setting_30fps_VGA[] = {
> > >   {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0},

[PATCH v2 2/2] media: ov5640: Fix timings setup code

2018-07-10 Thread Jacopo Mondi
From: Samuel Bobrowicz 

The current code, when changing the mode and changing the scaling or
sampling parameters, will look at the horizontal and vertical total size,
which, since 5999f381e023 ("media: ov5640: Add horizontal and vertical
totals") has been moved from the static register initialization to after
the mode change.

That means that the values are no longer set up before the code retrieves
them, which is obviously a bug.

In addition, restore timings settings in the initial configuration register
blob only, to have MIPI capture operations work again.

Fixes: 5999f381e023 ("media: ov5640: Add horizontal and vertical totals")
Signed-off-by: Samuel Bobrowicz 
Signed-off-by: Maxime Ripard 
[re-introduce timing parameters in initial configuration blob]
Signed-off-by: Jacopo Mondi 

---
Compared to Maxime's and Sam's original version, this one re-introduces timing
configuration parameters in the init_settings configuration blob.

On my testing platform this fixes MIPI capture operations, that with the
original patch version applied was failing anyway.

Thanks
   j
---
 drivers/media/i2c/ov5640.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 7bbd1d7..bbcb908 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -277,7 +277,9 @@ static const struct reg_value 
ov5640_init_setting_30fps_VGA[] = {
{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-   {0x3810, 0x00, 0, 0},
+   {0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
+   {0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
+   {0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -1484,6 +1486,10 @@ static int ov5640_set_mode_exposure_calc(struct 
ov5640_dev *sensor,
if (ret < 0)
return ret;

+   ret = ov5640_set_timings(sensor, mode);
+   if (ret < 0)
+   return ret;
+
/* read capture VTS */
ret = ov5640_get_vts(sensor);
if (ret < 0)
@@ -1611,6 +1617,10 @@ static int ov5640_set_mode_direct(struct ov5640_dev 
*sensor,
if (ret < 0)
return ret;

+   ret = ov5640_set_timings(sensor, mode);
+   if (ret < 0)
+   return ret;
+
/* turn auto gain/exposure back on for direct mode */
ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 1);
if (ret)
@@ -1658,10 +1668,6 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
if (ret < 0)
return ret;

-   ret = ov5640_set_timings(sensor, mode);
-   if (ret < 0)
-   return ret;
-
ret = ov5640_set_binning(sensor, dn_mode != SCALING);
if (ret < 0)
return ret;
--
2.7.4



[PATCH v2 1/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-10 Thread Jacopo Mondi
Rework the MIPI interface startup sequence with the following changes:

- Remove MIPI bus initialization from the initial settings blob
- At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in
  LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode.
- At s_stream time enable/disable the MIPI interface output.
- Restore default settings at set_power(0) time.

Before this commit the sensor MIPI interface was initialized with settings
that require a start/stop sequence at power-up time in order to force lanes
into LP11 state, as they were initialized in LP00 when in 'sleep mode',
which is assumed to be the sensor manual definition for the D-PHY defined
stop mode.

The stream start/stop was performed by enabling disabling clock gating,
and had the side effect to change the lanes sleep mode configuration when
stream was stopped.

Clock gating/ungating:
-   ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5),
-on ? 0 : BIT(5));
-   if (ret)

Set lanes in LP11 when in 'sleep mode':
-   ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
-  on ? 0x00 : 0x70);

This commit fixes an issue reported by Jagan Teki on i.MX6 platforms that
prevents the host interface from powering up correctly:
https://lkml.org/lkml/2018/6/1/38

It also improves MIPI capture operations stability on my testing platform
where MIPI capture often failed and returned all-purple frames.

fixes: f22996db44e2 ("media: ov5640: add support of DVP parallel interface")
Reported-by: Jagan Teki 
Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/ov5640.c | 91 --
 1 file changed, 71 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1ecbb7a..7bbd1d7 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -286,10 +286,10 @@ static const struct reg_value 
ov5640_init_setting_30fps_VGA[] = {
{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x3000, 0x00, 0, 0},
{0x3002, 0x1c, 0, 0}, {0x3004, 0xff, 0, 0}, {0x3006, 0xc3, 0, 0},
-   {0x300e, 0x45, 0, 0}, {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
+   {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
{0x501f, 0x00, 0, 0}, {0x4713, 0x03, 0, 0}, {0x4407, 0x04, 0, 0},
{0x440e, 0x00, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-   {0x4837, 0x0a, 0, 0}, {0x4800, 0x04, 0, 0}, {0x3824, 0x02, 0, 0},
+   {0x4837, 0x0a, 0, 0}, {0x3824, 0x02, 0, 0},
{0x5000, 0xa7, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x5180, 0xff, 0, 0},
{0x5181, 0xf2, 0, 0}, {0x5182, 0x00, 0, 0}, {0x5183, 0x14, 0, 0},
{0x5184, 0x25, 0, 0}, {0x5185, 0x24, 0, 0}, {0x5186, 0x09, 0, 0},
@@ -1102,12 +1102,18 @@ static int ov5640_set_stream_mipi(struct ov5640_dev 
*sensor, bool on)
 {
int ret;

-   ret = ov5640_mod_reg(sensor, OV5640_REG_MIPI_CTRL00, BIT(5),
-on ? 0 : BIT(5));
-   if (ret)
-   return ret;
-   ret = ov5640_write_reg(sensor, OV5640_REG_PAD_OUTPUT00,
-  on ? 0x00 : 0x70);
+   /*
+* Enable/disable the MIPI interface
+*
+* 0x300e = on ? 0x45 : 0x40
+* [7:5] = 001  : 2 data lanes mode
+* [4] = 0  : Power up MIPI HS Tx
+* [3] = 0  : Power up MIPI LS Rx
+* [2] = 1/0: MIPI interface enable/disable
+* [1:0] = 01/00: FIXME: 'debug'
+*/
+   ret = ov5640_write_reg(sensor, OV5640_REG_IO_MIPI_CTRL00,
+  on ? 0x45 : 0x40);
if (ret)
return ret;

@@ -1786,23 +1792,68 @@ static int ov5640_set_power(struct ov5640_dev *sensor, 
bool on)
if (ret)
goto power_off;

+   /* We're done here for DVP bus, while CSI-2 needs setup. */
+   if (sensor->ep.bus_type != V4L2_MBUS_CSI2)
+   return 0;
+
+   /*
+* Power up MIPI HS Tx and LS Rx; 2 data lanes mode
+*
+* 0x300e = 0x40
+* [7:5] = 001  : 2 data lanes mode
+* [4] = 0  : Power up MIPI HS Tx
+* [3] = 0  : Power up MIPI LS Rx
+* [2] = 0  : MIPI interface disabled
+*/
+   ret = ov5640_write_reg(sensor,
+  OV5640_REG_IO_MIPI_CTRL00, 0x40);
+   if (ret)
+   goto power_off;
+
+   /*
+* Gate clock and set LP11 in 'no packets mode' (idle)
+*
+* 0x4800 = 0x24
+* [5] = 1  : Gate clock when 'no packets'
+* [2] = 1  : MIPI bus in LP11 when 'no packets'
+*/
+   ret = ov5640_write_reg(sensor,
+ 

info!!

2018-07-10 Thread Lee Morrow
Top of the day to you, this is in respect of a very beneficial transaction 
which you would not want to let go reply for more details,
Regards,
Lee 



Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence

2018-07-10 Thread Steve Longerbeam

Hi Jacopo,

Sorry to report my testing on SabreSD has same result
as last time. This series fixes the LP-11 timeout at stream
on but captured images are still blank. I tried the 640x480
mode with UYVY2X8. Here is the pad config:

# media-ctl --get-v4l2 "'ov5640 1-003c':0"
        [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:full-range]


Steve

On 07/10/2018 11:36 AM, Jacopo Mondi wrote:

Hello,
this series fixes capture operations on i.MX6Q platforms (and possible other
platforms reported not working) using MIPI CSI-2 interface.

This iteration expands the v1 version with an additional fix, initially
submitted by Maxime in his series:
[PATCH v3 00/12] media: ov5640: Misc cleanup and improvements
https://www.spinics.net/lists/linux-media/msg134436.html

The original patch has been reported not fully fixing the issues by Daniel Mack
in his comment here below (on a Qualcomm platform if I'm not wrong):
https://www.spinics.net/lists/linux-media/msg134524.html
On my i.MX6Q testing platform that patch alone does not fix MIPI capture
neither.

The version I'm sending here re-introduces some of the timings parameters in the
initial configuration blob (not in the single mode ones), which apparently has
to be at least initially programmed to allow the driver to later program them
singularly in the 'set_timings()' function. Unfortunately I do not have a real
rationale behind this which explains why it has to be done this way :(

For the MIPI startup sequence re-work patch, no changes compared to v1.
Steve reported he has verified the LP-11 timout issue is solved on his testing
platform too. For more details, please refer to the v1 cover letter:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg133352.html

Thanks
j

Jacopo Mondi (1):
   media: i2c: ov5640: Re-work MIPI startup sequence

Samuel Bobrowicz (1):
   media: ov5640: Fix timings setup code

  drivers/media/i2c/ov5640.c | 107 ++---
  1 file changed, 82 insertions(+), 25 deletions(-)

--
2.7.4





Wine Enthusiasts List

2018-07-10 Thread Sofia Thomas



--
Hi,

Would you be interested in reaching out to "Wine Drinkers list " from USA?

Our Databases:-1.Beer Drinkers List 2.Alcohol Drinkers List

3.Beverage Consumers 4.Liquor  Drinkers List

5.Food Enthusiasts List  6.Gift Buyers List

7.Luxury Brand Buyers List   8.Home Owners List

9.Spa and Resort Visitors and many more.

All the contacts are opt-in verified, 100 percent permission based and can be 
used for unlimited multi-channel marketing.

Please let me know your thoughts towards procuring the Wine Drinkers list.

Best Regards,
Sofia Thoma

We respect your privacy, if you do not wish to receive any further emails from 
our end, please reply with a subject ³Leave Out².



Re: [PATCH -next v3 2/2] media: ov772x: use SCCB helpers

2018-07-10 Thread Wolfram Sang

> > I think it would be even better to introduce a SSCB regmap layer
> > and use that.
> 
> I'm fine with introducing a SCCB regmap layer.

I am fine with this approach, too.

> But do we need to provide both a SCCB regmap and these SCCB helpers?

I don't know much about the OV sensor drivers. I'd think they would
benefit from regmap, so a-kind-of enforced conversion to regmap doesn't
sound too bad to me.

> If we have a regmap_init_sccb(), I feel these three SCCB helper functions
> don't need to be exported anymore.

Might be true. But other media guys are probably in a better position to
comment on this?

Note that I found SCCB devices with 16 bit regs: ov2659 & ov 2685. I
think we can cover those with SMBus word accesses. regmap is probably
also the cleanest way to hide this detail?



signature.asc
Description: PGP signature


Re: [PATCH v8 1/3] uvcvideo: remove a redundant check

2018-07-10 Thread Laurent Pinchart
Hi Guennadi,

Thank you for the patch.

On Tuesday, 8 May 2018 18:07:42 EEST Guennadi Liakhovetski wrote:
> Event subscribers cannot have a NULL file handle. They are only added
> at a single location in the code, and the .fh pointer is used without
> checking there.
> 
> Signed-off-by: Guennadi Liakhovetski 

Reviewed-by: Laurent Pinchart 

and applied to my tree. I'll proceed to patches 2/3 and 3/3 tomorrow 
(Wednesday).

> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> b/drivers/media/usb/uvc/uvc_ctrl.c index a36b4fb..2a213c8 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1229,9 +1229,9 @@ static void uvc_ctrl_send_event(struct uvc_fh *handle,
> uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, value, changes);
> 
>   list_for_each_entry(sev, &mapping->ev_subs, node) {
> - if (sev->fh && (sev->fh != &handle->vfh ||
> + if (sev->fh != &handle->vfh ||
>   (sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK) ||
> - (changes & V4L2_EVENT_CTRL_CH_FLAGS)))
> + (changes & V4L2_EVENT_CTRL_CH_FLAGS))
>   v4l2_event_queue_fh(sev->fh, &ev);
>   }
>  }

-- 
Regards,

Laurent Pinchart





cron job: media_tree daily build: OK

2018-07-10 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Wed Jul 11 05:00:11 CEST 2018
media-tree git hash:666e994aa2278e948e2492ee9d81b4df241e7222
media_build git hash:   f3b64e45d2f2ef45cd4ae5b90a8f2a4fb284e43c
v4l-utils git hash: 07ab94cbdd5b19725ae3e76ad9ca03bd42e4e963
edid-decode git hash:   ab18befbcacd6cd4dff63faa82e32700369d6f25
gcc version:i686-linux-gcc (GCC) 8.1.0
sparse version: 0.5.2
smatch version: 0.5.1
host hardware:  x86_64
host os:4.16.0-1-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-i686: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
Check COMPILE_TEST: OK
linux-2.6.36.4-i686: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-i686: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-i686: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-i686: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.101-i686: OK
linux-3.0.101-x86_64: OK
linux-3.1.10-i686: OK
linux-3.1.10-x86_64: OK
linux-3.2.101-i686: OK
linux-3.2.101-x86_64: OK
linux-3.3.8-i686: OK
linux-3.3.8-x86_64: OK
linux-3.4.113-i686: OK
linux-3.4.113-x86_64: OK
linux-3.5.7-i686: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-i686: OK
linux-3.6.11-x86_64: OK
linux-3.7.10-i686: OK
linux-3.7.10-x86_64: OK
linux-3.8.13-i686: OK
linux-3.8.13-x86_64: OK
linux-3.9.11-i686: OK
linux-3.9.11-x86_64: OK
linux-3.10.108-i686: OK
linux-3.10.108-x86_64: OK
linux-3.11.10-i686: OK
linux-3.11.10-x86_64: OK
linux-3.12.74-i686: OK
linux-3.12.74-x86_64: OK
linux-3.13.11-i686: OK
linux-3.13.11-x86_64: OK
linux-3.14.79-i686: OK
linux-3.14.79-x86_64: OK
linux-3.15.10-i686: OK
linux-3.15.10-x86_64: OK
linux-3.16.56-i686: OK
linux-3.16.56-x86_64: OK
linux-3.17.8-i686: OK
linux-3.17.8-x86_64: OK
linux-3.18.102-i686: OK
linux-3.18.102-x86_64: OK
linux-3.19.8-i686: OK
linux-3.19.8-x86_64: OK
linux-4.0.9-i686: OK
linux-4.0.9-x86_64: OK
linux-4.1.51-i686: OK
linux-4.1.51-x86_64: OK
linux-4.2.8-i686: OK
linux-4.2.8-x86_64: OK
linux-4.3.6-i686: OK
linux-4.3.6-x86_64: OK
linux-4.4.109-i686: OK
linux-4.4.109-x86_64: OK
linux-4.5.7-i686: OK
linux-4.5.7-x86_64: OK
linux-4.6.7-i686: OK
linux-4.6.7-x86_64: OK
linux-4.7.10-i686: OK
linux-4.7.10-x86_64: OK
linux-4.8.17-i686: OK
linux-4.8.17-x86_64: OK
linux-4.9.91-i686: OK
linux-4.9.91-x86_64: OK
linux-4.10.17-i686: OK
linux-4.10.17-x86_64: OK
linux-4.11.12-i686: OK
linux-4.11.12-x86_64: OK
linux-4.12.14-i686: OK
linux-4.12.14-x86_64: OK
linux-4.13.16-i686: OK
linux-4.13.16-x86_64: OK
linux-4.14.42-i686: OK
linux-4.14.42-x86_64: OK
linux-4.15.14-i686: OK
linux-4.15.14-x86_64: OK
linux-4.16.8-i686: OK
linux-4.16.8-x86_64: OK
linux-4.17.2-i686: OK
linux-4.17.2-x86_64: OK
linux-4.18-rc1-i686: OK
linux-4.18-rc1-x86_64: OK
apps: OK
spec-git: OK

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Wednesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html