RE: [PATCH] media: imx208: Add imx208 camera sensor driver
Hi Sakari,
Some of suggestions below has been added in to [PATCH v2] or [PATCH v3].
We will fix others in PATCH v4 soon.
Thanks,
PC
-Original Message-
From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com]
Sent: Monday, July 16, 2018 11:59 PM
To: Chen, Ping-chung
Cc: linux-media@vger.kernel.org; Yeh, Andy ;
tf...@chromium.org
Subject: Re: [PATCH] media: imx208: Add imx208 camera sensor driver
Hi Ping-chung,
Thanks for the patch. Please see my comments below.
On Tue, Jul 10, 2018 at 03:15:36PM +0800, Ping-chung Chen wrote:
> 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
s/-level//
> + 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
Please drop this.
> +
> +#define IMX208_REG_VALUE_08BIT 1
> +#define IMX208_REG_VALUE_16BIT 2
> +#define IMX208_REG_VALUE_24BIT 3
The last one is unused.
> +
> +#define IMX208_REG_MODE_SELECT 0x0100
> +#define IMX208_MODE_STANDBY 0x00
> +#define IMX208_MODE_STREAMING0x01
> +
> +/* 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_384MHZ1124
> +#define IMX208_PPL_96MHZ 1124
> +
> +/* Exposure control */
> +#define IMX208_REG_EXPOSURE 0x0202
> +#define IMX208_EXPOSURE_MIN 4
> +#define IMX208_EXPOSURE_STEP 1
> +#define IMX208_EXPOSURE_DEFAULT 0x190
> +#define IMX208_EXPOSURE_MAX 65535
> +
> +/* Analog gain control */
> +#define IMX208_REG_ANALOG_GAIN 0x0204
> +#define IMX208_ANA_GAIN_MIN 0
> +#define IMX208_ANA_GAIN_MAX 0x00e0
> +#define IMX208_ANA_GAIN_STEP 1
> +#define IMX208_ANA_GAIN_DEFAULT 0x0
> +
> +/* Digital gain control */
> +#define IMX208_REG_GR_DIGITAL_GAIN 0x020e
> +#define IMX208_REG_R_DIGITAL_GAIN0x0210
> +#define IMX208_REG_B_DIGITAL_GAIN0x0212
> +#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_CONTROL 0x0101
> +#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
cron job: media_tree daily build: OK
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: Tue Jul 17 05:00:10 CEST 2018 media-tree git hash:39fbb88165b2bbbc77ea7acab5f10632a31526e6 media_build git hash: f3b64e45d2f2ef45cd4ae5b90a8f2a4fb284e43c v4l-utils git hash: e4df0e3cd3a84570714defe279d13eae894cb1fa 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.102-i686: OK linux-3.2.102-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.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.115-i686: OK linux-3.18.115-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.52-i686: OK linux-4.1.52-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.140-i686: OK linux-4.4.140-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.112-i686: OK linux-4.9.112-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.55-i686: OK linux-4.14.55-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.6-i686: OK linux-4.17.6-x86_64: OK linux-4.18-rc4-i686: OK linux-4.18-rc4-x86_64: OK apps: OK spec-git: OK Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
images
We can process 400+ images per day. If you need any image editing, please let us know. Photos cut out; Photos clipping path; Photos masking; Photo shadow creation; Photos retouching; Beauty Model retouching on skin, face, body; Glamour retouching; Products retouching. We can give you testing for your photos. Turnaround time is fast Thanks, Simon
editing of your photos
We can process 400+ images per day. If you need any image editing, please let us know. Photos cut out; Photos clipping path; Photos masking; Photo shadow creation; Photos retouching; Beauty Model retouching on skin, face, body; Glamour retouching; Products retouching. We can give you testing for your photos. Turnaround time is fast Thanks, Simon
Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence
On 07/16/2018 01:29 AM, jacopo mondi wrote: Hi Steve, thanks for keep testing it On Sat, Jul 14, 2018 at 01:02:32PM -0700, Steve Longerbeam wrote: On 07/14/2018 12:41 PM, Steve Longerbeam wrote: Hi Jacopo, On 07/14/2018 11:57 AM, Steve Longerbeam wrote: Hi Jacopo, Pardon the late reply, see below. On 07/11/2018 12:21 AM, jacopo mondi wrote: Hi Steve, On Tue, Jul 10, 2018 at 02:10:54PM -0700, Steve Longerbeam wrote: 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: This saddens me :( I'm capturing with the same format and sizes... this shouldn't be the issue Could you confirm this matches what you have in your tree? 5dc2c80 media: ov5640: Fix timings setup code b35e757 media: i2c: ov5640: Re-work MIPI startup sequence 3c4a737 media: ov5640: fix frame interval enumeration 41cb1c7 media: ov5640: adjust xclk_max c3f3ba3 media: ov5640: add support of module orientation ce85705 media: ov5640: add HFLIP/VFLIP controls support 8663341 media: ov5640: Program the visible resolution 476dec0 media: ov5640: Add horizontal and vertical totals dba13a0 media: ov5640: Change horizontal and vertical resolutions name 8f57c2f media: ov5640: Init properly the SCLK dividers Yes, I have that commit sequence. FWIW, I can verify what Jagan Teki reported earlier, that the driver still works on the SabreSD platform at: dba13a0 media: ov5640: Change horizontal and vertical resolutions name and is broken at: 476dec0 media: ov5640: Add horizontal and vertical totals with LP-11 timeout at the mipi csi-2 receiver: [ 80.763189] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0230 [ 80.769599] ipu1_csi1: pipeline start failed with -110 And I discovered the bug in 476dec0 "media: ov5640: Add horizontal and vertical totals". The call to ov5640_set_timings() needs to be moved before the calls to ov5640_get_vts() and ov5640_get_hts(). But I see you have discovered that as well, and fixed in the second patch in your series. I'm sorry I'm not sur I'm following. Does this mean that with that bug you are referring to up here fixed by my last patch you have capture working? No, capture still not working for me on SabreSD, even after fixing the bug in 476dec0 "media: ov5640: Add horizontal and vertical totals", by either using your patchset, or by running version 476dec0 of ov5640.c with the call to ov5640_set_timings() moved to the correct places as described below. Steve But strangely, if I revert to 476dec0, and then move the call to ov5640_set_timings() to just after ov5640_load_regs() in ov5640_set_mode_exposure_calc() and ov5640_set_mode_direct(), the LP-11 timeouts are still present. So I can confirm this strangeness which you already pointed out below [1]. 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 :( [1] here :) Steve
Re: [PATCH] media: coda: add SPS fixup code for frame sizes that are not multiples of 16
On Mon, 2018-07-02 at 10:53 +0200, Hans Verkuil wrote:
> On 28/06/18 18:47, Philipp Zabel wrote:
> > The CODA firmware does not set the VUI frame cropping fields to properly
> > describe coded h.264 streams with frame sizes that are not a multiple of
> > the macroblock size.
> > This adds RBSP parsing code and a SPS fixup routine to manually replace
> > the cropping information in the headers produced by the firmware with
> > the correct values.
> >
> > Signed-off-by: Philipp Zabel
> > ---
> > drivers/media/platform/coda/coda-bit.c | 11 +
> > drivers/media/platform/coda/coda-h264.c | 302
> > drivers/media/platform/coda/coda.h | 2 +
> > 3 files changed, 315 insertions(+)
> >
> > diff --git a/drivers/media/platform/coda/coda-bit.c
> > b/drivers/media/platform/coda/coda-bit.c
> > index 94ba3cc3bf14..d6380a10fa2c 100644
> > --- a/drivers/media/platform/coda/coda-bit.c
> > +++ b/drivers/media/platform/coda/coda-bit.c
> > @@ -1197,6 +1197,17 @@ static int coda_start_encoding(struct coda_ctx *ctx)
> > if (ret < 0)
> > goto out;
> >
> > + if ((q_data_src->rect.width % 16) ||
> > + (q_data_src->rect.height % 16)) {
> > + ret = coda_sps_fixup(ctx, q_data_src->rect.width,
> > +q_data_src->rect.height,
> > +&ctx->vpu_header[0][0],
> > +&ctx->vpu_header_size[0],
> > +sizeof(ctx->vpu_header[0]));
>
> You need to add a comment here why this is needed.
[...]
> > +int coda_sps_fixup(struct coda_ctx *ctx, int width, int height, char *buf,
> > + int *size, int max_size)
>
> Same here: why it is needed and what does it do.
Thank you, I'll send a v2 with this fixed.
This function rewrites the h.264 SPS header to set the crop_right and
crop_bottom fields correctly, as these are always set to 0 by the
CODA7541 firmware.
Setting the crop_* fields is important for frame sizes that are not
multiples of the macroblock size: those fields are used together with
the pic_width_in_mbs_minus1 and pic_height_in_map_units_minus1 fields
to determine the visible frame width and height.
regards
Philipp
Re: [PATCH] media: imx208: Add imx208 camera sensor driver
On Tue, Jul 10, 2018 at 07:37:54AM +, Yeh, Andy wrote:
> 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.
It seems the rest of the driver uses RGGB.
The enumeration should only contain what is possible using the current
flipping configuration.
--
Sakari Ailus
sakari.ai...@linux.intel.com
Re: [PATCH] media: imx208: Add imx208 camera sensor driver
Hi Ping-chung,
Thanks for the patch. Please see my comments below.
On Tue, Jul 10, 2018 at 03:15:36PM +0800, Ping-chung Chen wrote:
> 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
s/-level//
> + 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
Please drop this.
> +
> +#define IMX208_REG_VALUE_08BIT 1
> +#define IMX208_REG_VALUE_16BIT 2
> +#define IMX208_REG_VALUE_24BIT 3
The last one is unused.
> +
> +#define IMX208_REG_MODE_SELECT 0x0100
> +#define IMX208_MODE_STANDBY 0x00
> +#define IMX208_MODE_STREAMING0x01
> +
> +/* 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_384MHZ1124
> +#define IMX208_PPL_96MHZ 1124
> +
> +/* Exposure control */
> +#define IMX208_REG_EXPOSURE 0x0202
> +#define IMX208_EXPOSURE_MIN 4
> +#define IMX208_EXPOSURE_STEP 1
> +#define IMX208_EXPOSURE_DEFAULT 0x190
> +#define IMX208_EXPOSURE_MAX 65535
> +
> +/* Analog gain control */
> +#define IMX208_REG_ANALOG_GAIN 0x0204
> +#define IMX208_ANA_GAIN_MIN 0
> +#define IMX208_ANA_GAIN_MAX 0x00e0
> +#define IMX208_ANA_GAIN_STEP 1
> +#define IMX208_ANA_GAIN_DEFAULT 0x0
> +
> +/* Digital gain control */
> +#define IMX208_REG_GR_DIGITAL_GAIN 0x020e
> +#define IMX208_REG_R_DIGITAL_GAIN0x0210
> +#define IMX208_REG_B_DIGITAL_GAIN0x0212
> +#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_CONTROL 0x0101
> +#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 heigh
[PATCH 1/1] v4l: i2c: Replace "sensor-level" by "sensor"
A lot of sensor drivers are labelled as "sensor-level" drivers. That's odd and somewhat confusing as the term isn't used elsewhere: these are just sensor drivers. Call them such. Signed-off-by: Sakari Ailus --- drivers/media/i2c/Kconfig | 62 +++ 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 8669853e13614..5ee7a09001a7d 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -398,7 +398,7 @@ config VIDEO_TVP514X depends on VIDEO_V4L2 && I2C select V4L2_FWNODE ---help--- - This is a Video4Linux2 sensor-level driver for the TI TVP5146/47 + This is a Video4Linux2 sensor driver for the TI TVP5146/47 decoder. It is currently working with the TI OMAP3 camera controller. @@ -600,7 +600,7 @@ config VIDEO_IMX258 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 + This is a Video4Linux2 sensor driver for the Sony IMX258 camera. To compile this driver as a module, choose M here: the @@ -611,7 +611,7 @@ config VIDEO_IMX274 depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API depends on MEDIA_CAMERA_SUPPORT ---help--- - This is a V4L2 sensor-level driver for the Sony IMX274 + This is a V4L2 sensor driver for the Sony IMX274 CMOS image sensor. config VIDEO_OV2640 @@ -619,7 +619,7 @@ config VIDEO_OV2640 depends on VIDEO_V4L2 && I2C depends on MEDIA_CAMERA_SUPPORT help - This is a Video4Linux2 sensor-level driver for the OmniVision + This is a Video4Linux2 sensor driver for the OmniVision OV2640 camera. To compile this driver as a module, choose M here: the @@ -631,7 +631,7 @@ config VIDEO_OV2659 depends on MEDIA_CAMERA_SUPPORT select V4L2_FWNODE ---help--- - This is a Video4Linux2 sensor-level driver for the OmniVision + This is a Video4Linux2 sensor driver for the OmniVision OV2659 camera. To compile this driver as a module, choose M here: the @@ -643,7 +643,7 @@ config VIDEO_OV2685 depends on MEDIA_CAMERA_SUPPORT select V4L2_FWNODE ---help--- - This is a Video4Linux2 sensor-level driver for the OmniVision + This is a Video4Linux2 sensor driver for the OmniVision OV2685 camera. To compile this driver as a module, choose M here: the @@ -656,7 +656,7 @@ config VIDEO_OV5640 depends on MEDIA_CAMERA_SUPPORT select V4L2_FWNODE ---help--- - This is a Video4Linux2 sensor-level driver for the Omnivision + This is a Video4Linux2 sensor driver for the Omnivision OV5640 camera sensor with a MIPI CSI-2 interface. config VIDEO_OV5645 @@ -666,7 +666,7 @@ config VIDEO_OV5645 depends on MEDIA_CAMERA_SUPPORT select V4L2_FWNODE ---help--- - This is a Video4Linux2 sensor-level driver for the OmniVision + This is a Video4Linux2 sensor driver for the OmniVision OV5645 camera. To compile this driver as a module, choose M here: the @@ -678,7 +678,7 @@ config VIDEO_OV5647 depends on MEDIA_CAMERA_SUPPORT select V4L2_FWNODE ---help--- - This is a Video4Linux2 sensor-level driver for the OmniVision + This is a Video4Linux2 sensor driver for the OmniVision OV5647 camera. To compile this driver as a module, choose M here: the @@ -689,7 +689,7 @@ config VIDEO_OV6650 depends on I2C && VIDEO_V4L2 depends on MEDIA_CAMERA_SUPPORT ---help--- - This is a Video4Linux2 sensor-level driver for the OmniVision + This is a Video4Linux2 sensor driver for the OmniVision OV6650 camera. To compile this driver as a module, choose M here: the @@ -702,7 +702,7 @@ config VIDEO_OV5670 depends on MEDIA_CONTROLLER select V4L2_FWNODE ---help--- - This is a Video4Linux2 sensor-level driver for the OmniVision + This is a Video4Linux2 sensor driver for the OmniVision OV5670 camera. To compile this driver as a module, choose M here: the @@ -713,7 +713,7 @@ config VIDEO_OV5695 depends on I2C && VIDEO_V4L2 depends on MEDIA_CAMERA_SUPPORT ---help--- - This is a Video4Linux2 sensor-level driver for the OmniVision + This is a Video4Linux2 sensor driver for the OmniVision OV5695 camera. To compile this driver as a module, choose M here: the @@ -725,7 +725,7 @@ config VIDEO_OV7251 depends on MEDIA_CAMERA_SUPPORT select V4L2_FWNODE help - This is a Video4Linux2 sensor-level driver for the OmniVisio
editing
We can process 400+ images per day. If you need any image editing, please let us know. Photos cut out; Photos clipping path; Photos masking; Photo shadow creation; Photos retouching; Beauty Model retouching on skin, face, body; Glamour retouching; Products retouching. We can give you testing for your photos. Turnaround time is fast Thanks, Simon
Re: [PATCH 16/16] media: imx: add mem2mem device
Hi Steve,
On Thu, 2018-07-05 at 15:09 -0700, Steve Longerbeam wrote:
[...]
> > +static int mem2mem_try_fmt(struct file *file, void *priv,
> > + struct v4l2_format *f)
> > +{
[...]
> > + /*
> > +* Horizontally/vertically chroma subsampled formats must have even
> > +* width/height.
> > +*/
> > + switch (f->fmt.pix.pixelformat) {
> > + case V4L2_PIX_FMT_YUV420:
> > + case V4L2_PIX_FMT_YVU420:
> > + case V4L2_PIX_FMT_NV12:
> > + walign = 1;
> > + halign = 1;
> > + break;
> > + case V4L2_PIX_FMT_YUV422P:
> > + case V4L2_PIX_FMT_NV16:
> > + walign = 1;
> > + halign = 0;
> > + break;
> > + default:
>
> The default case should init walign, otherwise for OUTPUT direction,
> walign may not get initialized at all, see below...
Yes, thank you.
> > + halign = 0;
> > + break;
> > + }
> > + if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> > + /*
> > +* The IC burst reads 8 pixels at a time. Reading beyond the
> > +* end of the line is usually acceptable. Those pixels are
> > +* ignored, unless the IC has to write the scaled line in
> > +* reverse.
> > +*/
> > + if (!ipu_rot_mode_is_irt(ctx->rot_mode) &&
> > + ctx->rot_mode && IPU_ROT_BIT_HFLIP)
> > + walign = 3;
>
> This looks wrong. Do you mean:
>
> if (ipu_rot_mode_is_irt(ctx->rot_mode) || (ctx->rot_mode & IPU_ROT_BIT_HFLIP))
> walign = 3;
> else
> walign = 1;
The input DMA burst width alignment is only necessary if the lines are
scanned from right to left (that is, if HF is enabled) in the scaling
step.
If the rotator is used, the flipping is done in the rotation step
instead, so the alignment restriction would be on the width of the
intermediate tile (and thus on the output height). This is already
covered by the rotator 8x8 pixel block alignment.
> That is, require 8 byte width alignment for IRT or if HFLIP is enabled.
No, I specifically meant (!IRT && HFLIP).
The rotator itself doesn't cause any input alignment restrictions, we
just have to make sure that the intermediate tiles after scaling are 8x8
aligned.
> Also, why not simply call ipu_image_convert_adjust() in
> mem2mem_try_fmt()? If there is something missing in the former
> function, then it should be added there, instead of adding the
> missing checks in mem2mem_try_fmt().
ipu_image_convert_adjust tries to adjust both input and output image at
the same time, here we just have the format of either input or output
image. Do you suggest to split this function into an input and an output
version?
regards
Philipp
Re: [PATCH 16/16] media: imx: add mem2mem device
Hi Pavel, On Tue, 2018-07-10 at 14:07 +0200, Pavel Machek wrote: [...] > > +// 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. Thank you, I'll fix this in v2. regards Philipp
Re: [PATCH 00/16] i.MX media mem2mem scaler
Hi Steve,
On Thu, 2018-07-05 at 14:55 -0700, Steve Longerbeam wrote:
> Hi Philipp,
>
> Thanks for this great patchset! Finally we have improved seams
> with tiled conversions, and relaxed width alignment requirements.
>
> Unfortunately this patchset isn't working correctly yet. It breaks tiled
> conversions with rotation.
>
> Trying the following conversion:
>
> input: 720x480, UYVY
> output: 1280x768, UYVY, rotation=90 degrees
>
> causes non-8-byte aligned tile buffers at the output:
>
> [ 129.578210] imx-ipuv3 240.ipu: task 2: ctx 8955dec9: Input@[0,0]:
> phys
> [ 129.585980] imx-ipuv3 240.ipu: task 2: ctx 8955dec9: Input@[1,0]:
> phys 00051360
> [ 129.593736] imx-ipuv3 240.ipu: task 2: ctx 8955dec9:
> Output@[0,0]: phys
> [ 129.601556] imx-ipuv3 240.ipu: task 2: ctx 8955dec9:
> Output@[0,1]: phys 052e
>
> resulting in hung conversion and abort timeout:
>
> [ 147.689220] imx-ipuv3 240.ipu: ipu_image_convert_abort: timeout
>
> Note that when converting to a planar format, the final (rotated) chroma
> tile
> buffers are also mis-aligned, in addition to the Y buffers.
Ouch, thanks. Calculation of the allow_overshoot parameter to
find_best_seam() is inverted and the seam alignment does not
properly switch output width/height in the IRT case.
I'll fix this in v2:
--8<--
diff --git a/drivers/gpu/ipu-v3/ipu-image-convert.c
b/drivers/gpu/ipu-v3/ipu-image-convert.c
index 4eb76714505a..8e125def4f5c 100644
--- a/drivers/gpu/ipu-v3/ipu-image-convert.c
+++ b/drivers/gpu/ipu-v3/ipu-image-convert.c
@@ -683,6 +683,11 @@ static void find_seams(struct ipu_image_convert_ctx *ctx,
if (ipu_rot_mode_is_irt(ctx->rot_mode)) {
resized_width = out->base.rect.height;
resized_height = out->base.rect.width;
+ out_left_align = tile_top_align(out->fmt);
+ out_top_align = tile_left_align(out->fmt);
+ out_width_align = tile_height_align(out->type,
+ ctx->rot_mode);
+ out_height_align = tile_width_align(out->fmt);
out_right = out->base.rect.height;
out_bottom = out->base.rect.width;
}
@@ -732,7 +737,7 @@ static void find_seams(struct ipu_image_convert_ctx *ctx,
}
for (col = in->num_cols - 1; col > 0; col--) {
- bool allow_overshoot = (col == in->num_cols - 1) &&
+ bool allow_overshoot = (col < in->num_cols - 1) &&
!(ctx->rot_mode & IPU_ROT_BIT_HFLIP);
unsigned int out_start;
unsigned int out_end;
@@ -775,6 +780,7 @@ static void find_seams(struct ipu_image_convert_ctx *ctx,
in_right, flipped_out_left, out_right);
for (row = in->num_rows - 1; row > 0; row--) {
+ bool allow_overshoot = row < in->num_rows - 1;
unsigned int out_start;
unsigned int out_end;
unsigned int in_top;
@@ -788,8 +794,7 @@ static void find_seams(struct ipu_image_convert_ctx *ctx,
find_best_seam(ctx, out_start, out_end,
in_top_align, out_top_align, out_height_align,
ctx->downsize_coeff_v, ctx->image_resize_coeff_v,
- row == in->num_rows - 1,
- &in_top, &out_top);
+ allow_overshoot, &in_top, &out_top);
if ((ctx->rot_mode & IPU_ROT_BIT_VFLIP) ^
ipu_rot_mode_is_irt(ctx->rot_mode))
-->8--
regards
Philipp
Re: [PATCH] videobuf2-core: check for q->error in vb2_core_qbuf()
Hi Hans,
On Thu, Jul 05, 2018 at 10:25:19AM +0200, Hans Verkuil wrote:
> The vb2_core_qbuf() function didn't check if q->error was set. It is checked
> in
> __buf_prepare(), but that function isn't called if the buffer was already
> prepared before with VIDIOC_PREPARE_BUF.
>
> So check it at the start of vb2_core_qbuf() as well.
>
> Signed-off-by: Hans Verkuil
> ---
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index d3501cd604cb..5d7946ec80d8 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1484,6 +1484,11 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int
> index, void *pb,
> struct vb2_buffer *vb;
> int ret;
>
> + if (q->error) {
> + dprintk(1, "fatal error occurred on queue\n");
> + return -EIO;
> + }
> +
> vb = q->bufs[index];
>
> if ((req && q->uses_qbuf) ||
How long has this problem existed? It looks like something that should go
to the stable branches, too...
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Re: [PATCH v2 0/2] media: i2c: ov5640: Re-work MIPI startup sequence
Hi Steve, thanks for keep testing it On Sat, Jul 14, 2018 at 01:02:32PM -0700, Steve Longerbeam wrote: > > > On 07/14/2018 12:41 PM, Steve Longerbeam wrote: > >Hi Jacopo, > > > > > >On 07/14/2018 11:57 AM, Steve Longerbeam wrote: > >>Hi Jacopo, > >> > >>Pardon the late reply, see below. > >> > >>On 07/11/2018 12:21 AM, jacopo mondi wrote: > >>>Hi Steve, > >>> > >>>On Tue, Jul 10, 2018 at 02:10:54PM -0700, Steve Longerbeam wrote: > 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: > >>>This saddens me :( > >>> > >>>I'm capturing with the same format and sizes... this shouldn't be the > >>>issue > >>> > >>>Could you confirm this matches what you have in your tree? > >>>5dc2c80 media: ov5640: Fix timings setup code > >>>b35e757 media: i2c: ov5640: Re-work MIPI startup sequence > >>>3c4a737 media: ov5640: fix frame interval enumeration > >>>41cb1c7 media: ov5640: adjust xclk_max > >>>c3f3ba3 media: ov5640: add support of module orientation > >>>ce85705 media: ov5640: add HFLIP/VFLIP controls support > >>>8663341 media: ov5640: Program the visible resolution > >>>476dec0 media: ov5640: Add horizontal and vertical totals > >>>dba13a0 media: ov5640: Change horizontal and vertical resolutions name > >>>8f57c2f media: ov5640: Init properly the SCLK dividers > >> > >>Yes, I have that commit sequence. > >> > >>FWIW, I can verify what Jagan Teki reported earlier, that the driver > >>still > >>works on the SabreSD platform at: > >> > >>dba13a0 media: ov5640: Change horizontal and vertical resolutions name > >> > >>and is broken at: > >> > >>476dec0 media: ov5640: Add horizontal and vertical totals > >> > >>with LP-11 timeout at the mipi csi-2 receiver: > >> > >>[ 80.763189] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x0230 > >>[ 80.769599] ipu1_csi1: pipeline start failed with -110 > > > >And I discovered the bug in 476dec0 "media: ov5640: Add horizontal and > >vertical totals". The call to ov5640_set_timings() needs to be moved > >before the > >calls to ov5640_get_vts() and ov5640_get_hts(). But I see you have > >discovered > >that as well, and fixed in the second patch in your series. > > I'm sorry I'm not sur I'm following. Does this mean that with that bug you are referring to up here fixed by my last patch you have capture working? Thanks j > > But strangely, if I revert to 476dec0, and then move the call to > ov5640_set_timings() > to just after ov5640_load_regs() in ov5640_set_mode_exposure_calc() and > ov5640_set_mode_direct(), the LP-11 timeouts are still present. So I can > confirm > this strangeness which you already pointed out below [1]. > > > > > >> > >>> > > > > >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 :( > > > > [1] here :) > > Steve > > signature.asc Description: PGP signature
