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: Fri Sep 7 04:00:14 CEST 2018 media-tree git hash:d842a7cf938b6e0f8a1aa9f1aec0476c9a599310 media_build git hash: ed1d887e2c18299383c7258615130197c8ce4946 v4l-utils git hash: a8c766b75f82d0facbeed5a392d91f4469ffd716 edid-decode git hash: b2da1516df3cc2756bfe8d1fa06d7bf2562ba1f4 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: v0.5.0-3428-gdfe27cf host hardware: x86_64 host os:4.17.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.119-i686: OK linux-3.18.119-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.152-i686: OK linux-4.4.152-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.124-i686: OK linux-4.9.124-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.67-i686: OK linux-4.14.67-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.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.5-i686: OK linux-4.18.5-x86_64: OK linux-4.19-rc1-i686: OK linux-4.19-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH] Documentation/media: uapi: Explicitly say there are no Invariant Sections
Are you still waiting for agreement from any contributors, or is this ready to apply? Ben. On Fri, 2018-08-03 at 15:41 +0100, Ben Hutchings wrote: > The GNU Free Documentation License allows for a work to specify > Invariant Sections that are not allowed to be modified. (Debian > considers that this makes such works non-free.) > > The Linux Media Infrastructure userspace API documentation does not > specify any such sections, but it also doesn't say there are none (as > is recommended by the license text). Make it explicit that there are > none. > > References: https://bugs.debian.org/698668 > Signed-off-by: Ben Hutchings > --- > Documentation/media/media_uapi.rst | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Documentation/media/media_uapi.rst > b/Documentation/media/media_uapi.rst > index 28eb35a1f965..5198ff24a094 100644 > --- a/Documentation/media/media_uapi.rst > +++ b/Documentation/media/media_uapi.rst > @@ -10,9 +10,9 @@ Linux Media Infrastructure userspace API > > Permission is granted to copy, distribute and/or modify this document > under the terms of the GNU Free Documentation License, Version 1.1 or > -any later version published by the Free Software Foundation. A copy of > -the license is included in the chapter entitled "GNU Free Documentation > -License". > +any later version published by the Free Software Foundation, with no > +Invariant Sections. A copy of the license is included in the chapter > +entitled "GNU Free Documentation License". > > .. only:: html > -- Ben Hutchings I'm always amazed by the number of people who take up solipsism because they heard someone else explain it. - E*Borg on alt.fan.pratchett signature.asc Description: This is a digitally signed message part
[PATCH 2/2] au0828: Fix incorrect error messages
Correcting red herring error messages.
Where appropriate, replaces au0282_dev_register with:
- au0828_analog_register
- au0828_dvb_register
Signed-off-by: Brad Love
---
drivers/media/usb/au0828/au0828-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/au0828/au0828-core.c
b/drivers/media/usb/au0828/au0828-core.c
index 257ae0d..5abab1eb 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -626,7 +626,7 @@ static int au0828_usb_probe(struct usb_interface *interface,
/* Analog TV */
retval = au0828_analog_register(dev, interface);
if (retval) {
- pr_err("%s() au0282_dev_register failed to register on V4L2\n",
+ pr_err("%s() au0828_analog_register failed to register on
V4L2\n",
__func__);
mutex_unlock(&dev->lock);
goto done;
@@ -635,7 +635,7 @@ static int au0828_usb_probe(struct usb_interface *interface,
/* Digital TV */
retval = au0828_dvb_register(dev);
if (retval)
- pr_err("%s() au0282_dev_register failed\n",
+ pr_err("%s() au0828_dvb_register failed\n",
__func__);
/* Remote controller */
--
2.7.4
[PATCH 1/2] au0828: cannot kfree dev before usb disconnect
If au0828_analog_register fails, the dev is kfree'd and then flow
jumps to done, which can call au0828_usb_disconnect. Since all USB
error codes are negative, au0828_usb_disconnect will be called. The
problem is au0828_usb_disconnect uses dev, if dev is NULL then there
is immediate oops encountered.
[7.454307] au0828: au0828_usb_probe() au0282_dev_register failed to
register on V4L2
[7.454323] BUG: unable to handle kernel NULL pointer dereference at
0050
[7.454421] PGD 0 P4D 0
[7.454457] Oops: 0002 [#1] SMP PTI
[7.454500] CPU: 1 PID: 262 Comm: systemd-udevd Tainted: P O
4.18.3 #1
[7.454584] Hardware name: Google Panther/Panther, BIOS MattDevo 04/27/2015
[7.454670] RIP: 0010:_raw_spin_lock_irqsave+0x2c/0x50
[7.454725] Code: 44 00 00 55 48 89 e5 41 54 53 48 89 fb 9c 58 0f 1f 44 00
00 49 89 c4 fa 66 0f 1f 44 00 00 e8 db 23 1b ff 31 c0 ba 01 00 00 00 0f b1
13 85 c0 75 08 4c 89 e0 5b 41 5c 5d c3 89 c6 48 89 df e8
[7.455004] RSP: 0018:9130f53ef988 EFLAGS: 00010046
[7.455063] RAX: RBX: 0050 RCX:
[7.455139] RDX: 0001 RSI: 0003 RDI: 0050
[7.455216] RBP: 9130f53ef998 R08: 0018 R09: 0090
[7.455292] R10: ed4cc53cb000 R11: ed4cc53cb108 R12: 0082
[7.455369] R13: 9130cf2c6188 R14: R15: 0018
[7.455447] FS: 7f2ff8514cc0() GS:9130fcb0()
knlGS:
[7.455535] CS: 0010 DS: ES: CR0: 80050033
[7.455597] CR2: 0050 CR3: 0001753f0002 CR4: 000606a0
[7.455675] Call Trace:
[7.455713] __wake_up_common_lock+0x65/0xc0
[7.455764] __wake_up+0x13/0x20
[7.455808] ir_lirc_unregister+0x57/0xe0 [rc_core]
[7.455865] rc_unregister_device+0xa0/0xc0 [rc_core]
[7.455935] au0828_rc_unregister+0x25/0x40 [au0828]
[7.455999] au0828_usb_disconnect+0x33/0x80 [au0828]
[7.456064] au0828_usb_probe.cold.16+0x8d/0x2aa [au0828]
[7.456130] usb_probe_interface+0xf1/0x300
[7.456184] driver_probe_device+0x2e3/0x460
[7.456235] __driver_attach+0xe4/0x110
[7.456282] ? driver_probe_device+0x460/0x460
[7.456335] bus_for_each_dev+0x74/0xb0
[7.456385] ? kmem_cache_alloc_trace+0x15d/0x1d0
[7.456441] driver_attach+0x1e/0x20
[7.456485] bus_add_driver+0x159/0x230
[7.456532] driver_register+0x70/0xc0
[7.456578] usb_register_driver+0x7f/0x140
[7.456626] ? 0xc0474000
[7.456674] au0828_init+0xbc/0x1000 [au0828]
[7.456725] do_one_initcall+0x4a/0x1c9
[7.456771] ? _cond_resched+0x19/0x30
[7.456817] ? kmem_cache_alloc_trace+0x15d/0x1d0
[7.456873] do_init_module+0x60/0x210
[7.456918] load_module+0x221b/0x2710
[7.456966] ? vfs_read+0xf5/0x120
[7.457010] __do_sys_finit_module+0xbd/0x120
[7.457061] ? __do_sys_finit_module+0xbd/0x120
[7.457115] __x64_sys_finit_module+0x1a/0x20
[7.457166] do_syscall_64+0x5b/0x110
[7.457210] entry_SYSCALL_64_after_hwframe+0x49/0xbe
Signed-off-by: Brad Love
---
drivers/media/usb/au0828/au0828-core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/usb/au0828/au0828-core.c
b/drivers/media/usb/au0828/au0828-core.c
index cd363a2..257ae0d 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -629,7 +629,6 @@ static int au0828_usb_probe(struct usb_interface *interface,
pr_err("%s() au0282_dev_register failed to register on V4L2\n",
__func__);
mutex_unlock(&dev->lock);
- kfree(dev);
goto done;
}
--
2.7.4
[PATCH 0/2] au0828 Oops fix and message correction
An end user reported oopsing on connection of multiple Hauppauge 950Q devices. The oops is related to analog setup failing during usb probe for a device. If analog setup fails then dev is kfree'd and then drivers usb disconnect function is called, which requires valid dev. Hence, immediate oops. Patch 2 is just error message correction around the failure spot. An invalid function name is corrected. Brad Love (2): au0828: cannot kfree dev before usb disconnect au0828: Fix incorrect error messages drivers/media/usb/au0828/au0828-core.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- 2.7.4
Re: [PATCH 2/2] media: ov5640: Fix auto-exposure disabling
Hello,
On Tuesday, 14 August 2018 18:45:25 EEST jacopo mondi wrote:
> On Tue, Aug 07, 2018 at 08:53:23AM +, Hugues FRUCHET wrote:
> > Hi Jacopo,
> >
> > In serie "[PATCH 0/5] Fix OV5640 exposure & gain"
> > https://www.mail-archive.com/linux-media@vger.kernel.org/msg133269.html
> > I've tried to collect fixes around exposure/gain, not only the exposure
> > regression and I would prefer to keep it consistent with the associated
> > procedure test.
>
> You're right. Please see my other reply, I mixed two different issues
> in this series probably.
>
> > Moreover I dislike the internal use of control framework functions to
> > disable/enable exposure/gain, on my opinion this has to be kept simpler
> > by just disabling/enabling the right registers.
>
> Why that? I thought changing parameters exposed as controls should go
> through the control framework to ensure consistency. Maybe I'm wrong.
If I understand the driver correctly, auto-exposure has to be disabled
temporarily when changing format and size, due to internal hardware
requirements. The sequence should more or less be
1. Disable auto-exposure
2. Configure the format and size
3. Restore auto-exposure
This sequence is internal to the driver, and should thus not be visible to
userspace. Going through the control framework to disable and restore auto-
exposure would generate control events that would just confuse userspace. For
that reason I'd keep all this internal with direct register access instead of
going through the control framework.
> > Would it be possible that you test my 5 patches serie on your side ?
>
> I did. I re-based the series on top of my MIPI and timings fixes and
> it actually solves the exposure issues I didn't know I had :)
>
> I'll comment on v2 as well as soon as I'll get an answer from Steve on
> the CSI-2 issue.
>
> > On 07/18/2018 03:04 PM, jacopo mondi wrote:
> > > Hi again,
> > >
> > > On Wed, Jul 18, 2018 at 01:19:03PM +0200, Jacopo Mondi wrote:
> > >> As of:
> > >> commit bf4a4b518c20 ("media: ov5640: Don't force the auto exposure
> > >> state at
> > >> start time") auto-exposure got disabled before programming new capture
> > >> modes to the sensor. Unfortunately the function used to do that
> > >> (ov5640_set_exposure()) does not enable/disable auto-exposure engine
> > >> through register 0x3503[0] bit, but programs registers [0x3500 -
> > >> 0x3502] which represent the desired exposure time when running with
> > >> manual exposure. As a result, auto-exposure was not actually disabled
> > >> at all.
> > >>
> > >> To actually disable auto-exposure, go through the control framework
> > >> instead of calling ov5640_set_exposure() function directly.
> > >>
> > >> Also, as auto-gain and auto-exposure are disabled un-conditionally but
> > >> only
> > >> restored to their previous values in ov5640_set_mode_direct() function,
> > >> move controls restoring so that their value is re-programmed
> > >> opportunely after either ov5640_set_mode_direct() or
> > >> ov5640_set_mode_exposure_calc() have been executed.
> > >>
> > >> Fixes: bf4a4b518c20 ("media: ov5640: Don't force the auto exposure
> > >> state at start time") Signed-off-by: Jacopo Mondi
> > >>
> > >> ---
> > >> Is it worth doing with auto-gain what we're doing with auto-exposure?
> > >> Cache the value and then re-program it instead of unconditionally
> > >> disable/enable it?> >
> > > I have missed this patch from Hugues that address almost the same
> > > issue
> > > https://www.mail-archive.com/linux-media@vger.kernel.org/msg133264.html
> > >
> > > I feel this new one is simpler, and unless we want to avoid going
> > > through the control framework, it is not worth adding new functions to
> > > handle auto-exposure as Hugues' patch is doing.
> > >
> > > Hugues, do you have comments? Feel free to add your sob or rb tags if
> > > you like to.
> > >
> > > Thanks
> > >
> > > j
> > >>
> > >> Thanks
> > >>
> > >>j
> > >>
> > >> ---
> > >> ---
> > >>
> > >> drivers/media/i2c/ov5640.c | 29 +
> > >> 1 file changed, 13 insertions(+), 16 deletions(-)
> > >>
> > >> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > >> index 12b3496..bc75cb7 100644
> > >> --- a/drivers/media/i2c/ov5640.c
> > >> +++ b/drivers/media/i2c/ov5640.c
> > >> @@ -1588,25 +1588,13 @@ static int ov5640_set_mode_exposure_calc(struct
> > >> ov5640_dev *sensor,> >>
> > >>* change mode directly
> > >>*/
> > >>
> > >> static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
> > >>
> > >> - const struct ov5640_mode_info *mode,
> > >> - s32 exposure)
> > >> + const struct ov5640_mode_info *mode)
> > >>
> > >> {
> > >>
> > >> -int ret;
> > >> -
> > >>
> > >> if (!mode->reg_data)
> > >>
> > >> return -EINVAL;
> > >>
> > >> /* Write cap
Re: [PATCH v2 4/5] media: ov5640: fix auto controls values when switching to manual mode
Hi Hugues,
Thank you for the patch.
On Monday, 13 August 2018 13:19:45 EEST Hugues Fruchet wrote:
> When switching from auto to manual mode, V4L2 core is calling
> g_volatile_ctrl() in manual mode in order to get the manual initial value.
> Remove the manual mode check/return to not break this behaviour.
>
> Signed-off-by: Hugues Fruchet
> ---
> drivers/media/i2c/ov5640.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 9fb17b5..c110a6a 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -2277,16 +2277,12 @@ static int ov5640_g_volatile_ctrl(struct v4l2_ctrl
> *ctrl)
>
> switch (ctrl->id) {
> case V4L2_CID_AUTOGAIN:
> - if (!ctrl->val)
> - return 0;
> val = ov5640_get_gain(sensor);
> if (val < 0)
> return val;
> sensor->ctrls.gain->val = val;
> break;
What is this even supposed to do ? Only the V4L2_CID_GAIN and
V4L2_CID_EXPOSURE have the volatile flag set. Why can't this code be replaced
with
case V4L2_CID_GAIN:
val = ov5640_get_gain(sensor);
if (val < 0)
return val;
ctrl->val = val;
break;
> case V4L2_CID_EXPOSURE_AUTO:
> - if (ctrl->val == V4L2_EXPOSURE_MANUAL)
> - return 0;
> val = ov5640_get_exposure(sensor);
> if (val < 0)
> return val;
And same here.
--
Regards,
Laurent Pinchart
Re: [PATCH v2 1/5] media: ov5640: fix exposure regression
Hi Hugues,
Thank you for the patch.
On Monday, 13 August 2018 13:19:42 EEST Hugues Fruchet wrote:
> fixes: bf4a4b518c20 ("media: ov5640: Don't force the auto exposure state at
> start time").
>
> Symptom was black image when capturing HD or 5Mp picture
> due to manual exposure set to 1 while it was intended to
> set autoexposure to "manual", fix this.
>
> Signed-off-by: Hugues Fruchet
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/i2c/ov5640.c | 18 --
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 1ecbb7a..4b9da8b 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -938,6 +938,12 @@ static int ov5640_load_regs(struct ov5640_dev *sensor,
> return ret;
> }
>
> +static int ov5640_set_autoexposure(struct ov5640_dev *sensor, bool on)
> +{
> + return ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
> + BIT(0), on ? 0 : BIT(0));
> +}
> +
> /* read exposure, in number of line periods */
> static int ov5640_get_exposure(struct ov5640_dev *sensor)
> {
> @@ -1593,7 +1599,7 @@ static int ov5640_set_mode_exposure_calc(struct
> ov5640_dev *sensor, */
> static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
> const struct ov5640_mode_info *mode,
> - s32 exposure)
> + bool auto_exp)
> {
> int ret;
>
> @@ -1610,7 +1616,8 @@ static int ov5640_set_mode_direct(struct ov5640_dev
> *sensor, if (ret)
> return ret;
>
> - return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, exposure);
> + return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, auto_exp ?
> + V4L2_EXPOSURE_AUTO : V4L2_EXPOSURE_MANUAL);
> }
>
> static int ov5640_set_mode(struct ov5640_dev *sensor,
> @@ -1618,7 +1625,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
> {
> const struct ov5640_mode_info *mode = sensor->current_mode;
> enum ov5640_downsize_mode dn_mode, orig_dn_mode;
> - s32 exposure;
> + bool auto_exp = sensor->ctrls.auto_exp->val == V4L2_EXPOSURE_AUTO;
> int ret;
>
> dn_mode = mode->dn_mode;
> @@ -1629,8 +1636,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
> if (ret)
> return ret;
>
> - exposure = sensor->ctrls.auto_exp->val;
> - ret = ov5640_set_exposure(sensor, V4L2_EXPOSURE_MANUAL);
> + ret = ov5640_set_autoexposure(sensor, false);
> if (ret)
> return ret;
>
> @@ -1646,7 +1652,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
>* change inside subsampling or scaling
>* download firmware directly
>*/
> - ret = ov5640_set_mode_direct(sensor, mode, exposure);
> + ret = ov5640_set_mode_direct(sensor, mode, auto_exp);
> }
>
> if (ret < 0)
--
Regards,
Laurent Pinchart
Re: [PATCH v2 3/5] media: ov5640: fix wrong binning value in exposure calculation
Hi Hugues,
Thank you for the patch.
On Monday, 13 August 2018 13:19:44 EEST Hugues Fruchet wrote:
> ov5640_set_mode_exposure_calc() is checking binning value but
> binning value read is buggy, fix this.
> Rename ov5640_binning_on() to ov5640_get_binning() as per other
> similar functions.
>
> Signed-off-by: Hugues Fruchet
Reiewed-by: Laurent Pinchart
> ---
> drivers/media/i2c/ov5640.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 7c569de..9fb17b5 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -1349,7 +1349,7 @@ static int ov5640_set_ae_target(struct ov5640_dev
> *sensor, int target) return ov5640_write_reg(sensor, OV5640_REG_AEC_CTRL1F,
> fast_low); }
>
> -static int ov5640_binning_on(struct ov5640_dev *sensor)
> +static int ov5640_get_binning(struct ov5640_dev *sensor)
> {
> u8 temp;
> int ret;
> @@ -1357,8 +1357,8 @@ static int ov5640_binning_on(struct ov5640_dev
> *sensor) ret = ov5640_read_reg(sensor, OV5640_REG_TIMING_TC_REG21, &temp);
> if (ret)
> return ret;
> - temp &= 0xfe;
> - return temp ? 1 : 0;
> +
> + return temp & BIT(0);
> }
>
> static int ov5640_set_binning(struct ov5640_dev *sensor, bool enable)
> @@ -1468,7 +1468,7 @@ static int ov5640_set_mode_exposure_calc(struct
> ov5640_dev *sensor, if (ret < 0)
> return ret;
> prev_shutter = ret;
> - ret = ov5640_binning_on(sensor);
> + ret = ov5640_get_binning(sensor);
> if (ret < 0)
> return ret;
> if (ret && mode->id != OV5640_MODE_720P_1280_720 &&
--
Regards,
Laurent Pinchart
Re: [PATCH v2] [RFC v2] v4l2: add support for colorspace conversion for video capture
On Thu, 2018-09-06 at 12:54 +0200, Hans Verkuil wrote: [...] > > The application usually doesn't need to know whether the driver changed > > the requested ycbcr_enc because it doesn't have CSC matrix support at > > all, or because it doesn't implement a specific conversion. And if the > > application needs to know for some reason, it can always check > > VIDIOC_ENUM_FMT. > > > > > I don't think so, but I think that > > > is already true for the existing flag V4L2_PIX_FMT_FLAG_PREMUL_ALPHA. > > > > The only drivers using V4L2_PIX_FMT_FLAG_PREMUL_ALPHA I can see are > > vsp1_brx and vsp1_rpf. They never write to the v4l2_pix_format flags > > field. > > But they honor it. The problem is that I can set this flag and call S_FMT > on e.g. the vivid driver, and S_FMT will return the flag. But it actually > doesn't use the flag at all, so userspace has no way of knowing if the > flag is actually used. Userspace can see if its requested colorspace, etc. were changed by the driver, though. > Although it can call G_FMT and then the flag is cleared. So if drivers were to clear the flag, would they clear it if they don't support the flag at all, or also if they support the flag in principle, but can't convert into the specific requested value? Would all drivers have to be modified to clear those flags? I suppose rather the framework should be extended to set the flags on ENUM_FMT and to mask the TRY/S_FMT flags with those. > > > I wonder if V4L2_PIX_FMT_FLAG_PREMUL_ALPHA should also get an equivalent > > > flag for v4l2_fmtdesc. > > > > Isn't this useless to introduce after the fact, if there are already > > applications that use this feature? They can't depend on the existence > > of this flag to check for support anyway. > > Those applications are already hardcoded for the vsp1. So they won't break > by adding v4l2_fmtdesc flags. > > But apps like gstreamer and friends can start using these flags and deduce > what the HW is capable of. I see. In that case I agree. regards Philipp
Re: [PATCH v2] [RFC v2] v4l2: add support for colorspace conversion for video capture
On 09/06/18 11:50, Philipp Zabel wrote: > On Thu, 2018-09-06 at 11:02 +0200, Hans Verkuil wrote: >> Hi Philipp, >> >> It is much appreciated that this old RFC of mine > > Right, I should have made clearer that this is just a rework of Hans' > original RFC in [1]. > > [1] https://patchwork.linuxtv.org/patch/28847/ > >> is picked up again. I always wanted to get this in, but I never had a >> driver where it would make sense to do so. > > I'll test this with i.MX PXP and IPU mem2mem drivers and follow up with > per-driver patches to enable this feature once we know where this should > be going. > >> On 09/05/2018 07:09 PM, Philipp Zabel wrote: >>> For video capture it is the driver that reports the colorspace, >> >> add: "transfer function," > > Will do. > >>> Y'CbCr/HSV encoding and quantization range used by the video, and there >>> is no way to request something different, even though many HDTV >>> receivers have some sort of colorspace conversion capabilities. >>> >>> For output video this feature already exists since the application >>> specifies this information for the video format it will send out, and >>> the transmitter will enable any available CSC if a format conversion has >>> to be performed in order to match the capabilities of the sink. >>> >>> For video capture we propose adding new pix_format flags: >>> V4L2_PIX_FMT_FLAG_CSC_COLORSPACE, V4L2_PIX_FMT_FLAG_CSC_YCBCR_ENC, >>> V4L2_PIX_FMT_FLAG_CSC_HSV_ENC, V4L2_PIX_FMT_FLAG_CSC_QUANTIZATION, and >>> V4L2_PIX_FMT_FLAG_CSC_XFER_FUNC. These are set by the driver to indicate >>> its conversion features. When set by the application, the driver will >>> interpret the colorspace, ycbcr_enc/hsv_enc, quantization and xfer_func >>> fields as the requested colorspace information and will attempt to do >>> the conversion it supports. >>> >>> Drivers do not have to actually look at the flags: if the flags are not >>> set, then the colorspace, ycbcr_enc and quantization fields are set to >>> the default values by the core, i.e. just pass on the received format >>> without conversion. >> >> Thinking about this some more, I don't think this is quite the right >> approach. >> Having userspace set these flags with S_FMT if they want to do explicit >> conversions makes sense, and that part we can keep. >> >> But to signal the capabilities I think should be done via new flags for >> VIDIOC_ENUM_FMT. Basically the same set of flags, but for the flags field >> of struct v4l2_fmtdesc. > > In that case, I think the V4L2_PIX_FMT_FLAG_CSC_* should be purely a > signal from the application to the driver, and the driver should not > (have to) touch them at all. Right. The code in v4l2-ioctl.c that checks these flags and replaces the corresponding field with 0 (DEFAULT) would be sufficient. Drivers can just check the fields for non-default values. > An equivalent set of v4l2_fmtdesc flags could be used to signal > conversion support via VIDIOC_ENUM_FMT: > > #define V4L2_FMT_FLAG_CSC_COLORSPACE 0x0004 > #define V4L2_FMT_FLAG_CSC_YCBCR_ENC 0x0008 > #define V4L2_FMT_FLAG_CSC_HSV_ENC 0x0008 > #define V4L2_FMT_FLAG_CSC_QUANTIZATION0x0010 > #define V4L2_FMT_FLAG_CSC_XFER_FUNC 0x0020 > > What is the expected use case for these reported flags? Applications > that see them set to zero can skip enumerating capture side colorimetry. > Is there anything else? That's about it. It just signals if the HW is capable of doing such conversions. >> One thing that's not clear to me is what happens if userspace sets one or >> more flags and calls S_FMT for a driver that doesn't support this. Are the >> flags zeroed in that case upon return? > > I'd say no. Drivers are free to silently ignore the flag. > The effect is the same as if the driver supports the flag in principle, > but has to change a requested value anyway because of some limitation. > The application can check whether the driver changed its requested > colorspace, xfer_func, ycbcr_enc, or quantization. > > The application usually doesn't need to know whether the driver changed > the requested ycbcr_enc because it doesn't have CSC matrix support at > all, or because it doesn't implement a specific conversion. And if the > application needs to know for some reason, it can always check > VIDIOC_ENUM_FMT. > >> I don't think so, but I think that >> is already true for the existing flag V4L2_PIX_FMT_FLAG_PREMUL_ALPHA. > > The only drivers using V4L2_PIX_FMT_FLAG_PREMUL_ALPHA I can see are > vsp1_brx and vsp1_rpf. They never write to the v4l2_pix_format flags > field. But they honor it. The problem is that I can set this flag and call S_FMT on e.g. the vivid driver, and S_FMT will return the flag. But it actually doesn't use the flag at all, so userspace has no way of knowing if the flag is actually used. Although it can call G_FMT and then the flag is cleared. > >> I wonder if V4L2_PIX_FMT_FLAG_PREMUL_ALPHA should also get an equivalent >> flag for v4l2_fmtdesc. > > Isn't this useless to introduc
Re: [PATCH v3 1/4] dt-bindings: media: Add i.MX Pixel Pipeline binding
Hi Philippe, On Thu, 2018-09-06 at 12:36 +0200, Philippe De Muyter wrote: [...] > > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/fsl-pxp.txt > > @@ -0,0 +1,26 @@ > > +Freescale Pixel Pipeline > > + > > + > > +The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine > > +that supports scaling, colorspace conversion, alpha blending, rotation, and > > +pixel conversion via lookup table. Different versions are present on > > various > > +i.MX SoCs from i.MX23 to i.MX7. > > + > > +Required properties: > > +- compatible: should be "fsl,-pxp", where SoC can be one of imx23, > > imx28, > > + imx6dl, imx6sl, imx6ul, imx6sx, imx6ull, or imx7d. > > Is imx6q also compatible ? There is no pixel pipeline on i.MX6Q. It has the IPU image converter that can be used for scaling and colorspace conversion instead [1]. [1] https://patchwork.linuxtv.org/patch/51045/ regards Philipp
Re: [PATCH v3 1/4] dt-bindings: media: Add i.MX Pixel Pipeline binding
Hi Philipp, On Thu, Sep 06, 2018 at 11:02:12AM +0200, Philipp Zabel wrote: > Add DT binding documentation for the Pixel Pipeline (PXP) found on > various NXP i.MX SoCs. > > Signed-off-by: Philipp Zabel > Reviewed-by: Rob Herring > --- > No changes since v2. > --- > .../devicetree/bindings/media/fsl-pxp.txt | 26 +++ > 1 file changed, 26 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt > > diff --git a/Documentation/devicetree/bindings/media/fsl-pxp.txt > b/Documentation/devicetree/bindings/media/fsl-pxp.txt > new file mode 100644 > index ..2477e7f87381 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/fsl-pxp.txt > @@ -0,0 +1,26 @@ > +Freescale Pixel Pipeline > + > + > +The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine > +that supports scaling, colorspace conversion, alpha blending, rotation, and > +pixel conversion via lookup table. Different versions are present on various > +i.MX SoCs from i.MX23 to i.MX7. > + > +Required properties: > +- compatible: should be "fsl,-pxp", where SoC can be one of imx23, > imx28, > + imx6dl, imx6sl, imx6ul, imx6sx, imx6ull, or imx7d. Is imx6q also compatible ? Best regards Philippe -- Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles
Re: [PATCH 2/2] CNF4 pixel format for media subsystem
Hello Sergey,
On Thursday, 6 September 2018 12:37:36 EEST Hans Verkuil wrote:
> On 09/06/18 09:51, dorod...@gmail.com wrote:
> > From: Sergey Dorodnicov
> >
> > Registering new GUID used by Intel RealSense depth cameras with fourcc
> > CNF4, encoding sensor confidence information for every pixel.
s/confidence/depth confidence/
> >
> > Signed-off-by: Sergey Dorodnicov
> > Signed-off-by: Evgeni Raikhel
Could you please send me the output of lsusb -v (ideally run as root, or
through sudo) for a camera that uses this format ?
> > ---
> >
> > drivers/media/usb/uvc/uvc_driver.c | 5 +
> > drivers/media/usb/uvc/uvcvideo.h | 3 +++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c
> > b/drivers/media/usb/uvc/uvc_driver.c index d46dc43..c8d40a4 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -214,6 +214,11 @@ static struct uvc_format_desc uvc_fmts[] = {
> > .guid = UVC_GUID_FORMAT_INZI,
> > .fcc= V4L2_PIX_FMT_INZI,
> > },
> > + {
> > + .name = "Confidence 4-bit Packed (CNF4)",
>
> The name should correspond to what is set in v4l2-ioctl.c.
And should even be removed as it duplicates the names in v4l2-ioctl.c. I have
a half-baked patch to do so, I'll try to resurrect it. This isn't a blocking
issue, I'll rebase my patch on top of this one.
> > + .guid = UVC_GUID_FORMAT_CNF4,
> > + .fcc= V4L2_PIX_FMT_CNF4,
> > + },
> > };
> >
> > /* --
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h
> > b/drivers/media/usb/uvc/uvcvideo.h index e5f5d84..779bab2 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -154,6 +154,9 @@
> > #define UVC_GUID_FORMAT_INVI \
> > { 'I', 'N', 'V', 'I', 0xdb, 0x57, 0x49, 0x5e, \
> > 0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
> > +#define UVC_GUID_FORMAT_CNF4 \
> > + { 'C', ' ', ' ', ' ', 0x00, 0x00, 0x10, 0x00, \
> > +0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> >
> > #define UVC_GUID_FORMAT_D3DFMT_L8 \
> > {0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, \
--
Regards,
Laurent Pinchart
Re: [PATCH v2] [RFC v2] v4l2: add support for colorspace conversion for video capture
On Thu, 2018-09-06 at 11:02 +0200, Hans Verkuil wrote: > Hi Philipp, > > It is much appreciated that this old RFC of mine Right, I should have made clearer that this is just a rework of Hans' original RFC in [1]. [1] https://patchwork.linuxtv.org/patch/28847/ > is picked up again. I always wanted to get this in, but I never had a > driver where it would make sense to do so. I'll test this with i.MX PXP and IPU mem2mem drivers and follow up with per-driver patches to enable this feature once we know where this should be going. > On 09/05/2018 07:09 PM, Philipp Zabel wrote: > > For video capture it is the driver that reports the colorspace, > > add: "transfer function," Will do. > > Y'CbCr/HSV encoding and quantization range used by the video, and there > > is no way to request something different, even though many HDTV > > receivers have some sort of colorspace conversion capabilities. > > > > For output video this feature already exists since the application > > specifies this information for the video format it will send out, and > > the transmitter will enable any available CSC if a format conversion has > > to be performed in order to match the capabilities of the sink. > > > > For video capture we propose adding new pix_format flags: > > V4L2_PIX_FMT_FLAG_CSC_COLORSPACE, V4L2_PIX_FMT_FLAG_CSC_YCBCR_ENC, > > V4L2_PIX_FMT_FLAG_CSC_HSV_ENC, V4L2_PIX_FMT_FLAG_CSC_QUANTIZATION, and > > V4L2_PIX_FMT_FLAG_CSC_XFER_FUNC. These are set by the driver to indicate > > its conversion features. When set by the application, the driver will > > interpret the colorspace, ycbcr_enc/hsv_enc, quantization and xfer_func > > fields as the requested colorspace information and will attempt to do > > the conversion it supports. > > > > Drivers do not have to actually look at the flags: if the flags are not > > set, then the colorspace, ycbcr_enc and quantization fields are set to > > the default values by the core, i.e. just pass on the received format > > without conversion. > > Thinking about this some more, I don't think this is quite the right approach. > Having userspace set these flags with S_FMT if they want to do explicit > conversions makes sense, and that part we can keep. > > But to signal the capabilities I think should be done via new flags for > VIDIOC_ENUM_FMT. Basically the same set of flags, but for the flags field > of struct v4l2_fmtdesc. In that case, I think the V4L2_PIX_FMT_FLAG_CSC_* should be purely a signal from the application to the driver, and the driver should not (have to) touch them at all. An equivalent set of v4l2_fmtdesc flags could be used to signal conversion support via VIDIOC_ENUM_FMT: #define V4L2_FMT_FLAG_CSC_COLORSPACE0x0004 #define V4L2_FMT_FLAG_CSC_YCBCR_ENC 0x0008 #define V4L2_FMT_FLAG_CSC_HSV_ENC 0x0008 #define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0010 #define V4L2_FMT_FLAG_CSC_XFER_FUNC 0x0020 What is the expected use case for these reported flags? Applications that see them set to zero can skip enumerating capture side colorimetry. Is there anything else? > One thing that's not clear to me is what happens if userspace sets one or > more flags and calls S_FMT for a driver that doesn't support this. Are the > flags zeroed in that case upon return? I'd say no. Drivers are free to silently ignore the flag. The effect is the same as if the driver supports the flag in principle, but has to change a requested value anyway because of some limitation. The application can check whether the driver changed its requested colorspace, xfer_func, ycbcr_enc, or quantization. The application usually doesn't need to know whether the driver changed the requested ycbcr_enc because it doesn't have CSC matrix support at all, or because it doesn't implement a specific conversion. And if the application needs to know for some reason, it can always check VIDIOC_ENUM_FMT. > I don't think so, but I think that > is already true for the existing flag V4L2_PIX_FMT_FLAG_PREMUL_ALPHA. The only drivers using V4L2_PIX_FMT_FLAG_PREMUL_ALPHA I can see are vsp1_brx and vsp1_rpf. They never write to the v4l2_pix_format flags field. > I wonder if V4L2_PIX_FMT_FLAG_PREMUL_ALPHA should also get an equivalent > flag for v4l2_fmtdesc. Isn't this useless to introduce after the fact, if there are already applications that use this feature? They can't depend on the existence of this flag to check for support anyway. > Then we can just document that v4l2_format flags are only valid if they > are also defined in v4l2_fmtdesc. > > Does anyone have better ideas for this? I'd just say the driver is free to ignore the flag if it doesn't support the specific requested value and leave it at that. regards Philipp
Re: [PATCH 2/2] CNF4 pixel format for media subsystem
On 09/06/18 09:51, dorod...@gmail.com wrote:
> From: Sergey Dorodnicov
>
> Registering new GUID used by Intel RealSense depth cameras with fourcc
> CNF4, encoding sensor confidence information for every pixel.
>
> Signed-off-by: Sergey Dorodnicov
> Signed-off-by: Evgeni Raikhel
> ---
> drivers/media/usb/uvc/uvc_driver.c | 5 +
> drivers/media/usb/uvc/uvcvideo.h | 3 +++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c
> index d46dc43..c8d40a4 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -214,6 +214,11 @@ static struct uvc_format_desc uvc_fmts[] = {
> .guid = UVC_GUID_FORMAT_INZI,
> .fcc= V4L2_PIX_FMT_INZI,
> },
> + {
> + .name = "Confidence 4-bit Packed (CNF4)",
The name should correspond to what is set in v4l2-ioctl.c.
Regards,
Hans
> + .guid = UVC_GUID_FORMAT_CNF4,
> + .fcc= V4L2_PIX_FMT_CNF4,
> + },
> };
>
> /*
> diff --git a/drivers/media/usb/uvc/uvcvideo.h
> b/drivers/media/usb/uvc/uvcvideo.h
> index e5f5d84..779bab2 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -154,6 +154,9 @@
> #define UVC_GUID_FORMAT_INVI \
> { 'I', 'N', 'V', 'I', 0xdb, 0x57, 0x49, 0x5e, \
>0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
> +#define UVC_GUID_FORMAT_CNF4 \
> + { 'C', ' ', ' ', ' ', 0x00, 0x00, 0x10, 0x00, \
> + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
>
> #define UVC_GUID_FORMAT_D3DFMT_L8 \
> {0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, \
>
Re: [PATCH 1/2] CNF4 fourcc for 4 bit-per-pixel packed confidence information
Hi Sergey,
Some review comments:
On 09/06/18 09:51, dorod...@gmail.com wrote:
> From: Sergey Dorodnicov
>
> Adding new fourcc CNF4 for 4 bit-per-pixel packed confidence information
> provided by Intel RealSense depth cameras. Every two consecutive pixels
> are packed into a single byte (little-endian).
>
> Signed-off-by: Sergey Dorodnicov
> Signed-off-by: Evgeni Raikhel
> ---
> Documentation/media/uapi/v4l/depth-formats.rst | 1 +
> Documentation/media/uapi/v4l/pixfmt-cnf4.rst | 30
> ++
> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> include/uapi/linux/videodev2.h | 1 +
> 4 files changed, 33 insertions(+)
> create mode 100644 Documentation/media/uapi/v4l/pixfmt-cnf4.rst
>
> diff --git a/Documentation/media/uapi/v4l/depth-formats.rst
> b/Documentation/media/uapi/v4l/depth-formats.rst
> index d1641e9..9533348 100644
> --- a/Documentation/media/uapi/v4l/depth-formats.rst
> +++ b/Documentation/media/uapi/v4l/depth-formats.rst
> @@ -14,3 +14,4 @@ Depth data provides distance to points, mapped onto the
> image plane
>
> pixfmt-inzi
> pixfmt-z16
> +pixfmt-cnf4
> diff --git a/Documentation/media/uapi/v4l/pixfmt-cnf4.rst
> b/Documentation/media/uapi/v4l/pixfmt-cnf4.rst
> new file mode 100644
> index 000..d24fc1a
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-cnf4.rst
> @@ -0,0 +1,30 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _V4L2-PIX-FMT-CNF4:
> +
> +**
> +V4L2_PIX_FMT_CNF4 ('CNF4')
> +**
> +
> +Sensor confidence information as a 4 bits per pixel packed array
Confidence in what? I'll take a wild guess and say that it is about depth
confidence, but I don't think you actually say this anywhere :-)
> +
> +Description
> +===
> +
> +Proprietary format used by Intel RealSense Depth cameras containing sensor
> +confidence information in range 0-15 with 0 indicating that the sensor was
> +unable to resolve any signal and 15 indicating maximum level of confidence
> for
> +the specific sensor (actual error margins might change from sensor to
> sensor).
> +
> +Every two consecutive pixels are packed into a single byte (bit order is
> +little-endian).
Luckily, the bit order is always little-endian, so that doesn't help.
It's better to be specific here: bits 0-3 refer to the confidence value for the
pixel
at position X, bits 407 refer to the pixel at position X+1, where X is even.
> +
> +**Bit-packed representation.**
> +
> +.. flat-table::
> +:header-rows: 0
> +:stub-columns: 0
> +:widths: 64 64
> +
> +* - Y'\ :sub:`01[3:0]`\ (bits 3--0) Y'\ :sub:`00[3:0]`\ (bits 7--4)
> + - Y'\ :sub:`03[3:0]`\ (bits 3--0) Y'\ :sub:`02[3:0]`\ (bits 7--4)
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 54afc9c..eff296d 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1189,6 +1189,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> case V4L2_PIX_FMT_Y12I: descr = "Interleaved 12-bit Greyscale";
> break;
> case V4L2_PIX_FMT_Z16: descr = "16-bit Depth"; break;
> case V4L2_PIX_FMT_INZI: descr = "Planar 10:16 Greyscale Depth";
> break;
> + case V4L2_PIX_FMT_CNF4: descr = "4-bit Confidence (Packed)";
> break;
This should probably become "4-bit Depth Confidence (Packed)".
> case V4L2_PIX_FMT_PAL8: descr = "8-bit Palette"; break;
> case V4L2_PIX_FMT_UV8: descr = "8-bit Chrominance UV 4-4";
> break;
> case V4L2_PIX_FMT_YVU410: descr = "Planar YVU 4:1:0"; break;
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 5d1a368..a47f603 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -676,6 +676,7 @@ struct v4l2_pix_format {
> #define V4L2_PIX_FMT_Z16 v4l2_fourcc('Z', '1', '6', ' ') /* Depth data
> 16-bit */
> #define V4L2_PIX_FMT_MT21Cv4l2_fourcc('M', 'T', '2', '1') /* Mediatek
> compressed block mode */
> #define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel
> Planar Greyscale 10-bit and Depth 16-bit */
> +#define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit
> packed confidence information */
again, be explicit: "depth confidence"
>
> /* 10bit raw bayer packed, 32 bytes for every 25 pixels, last LSB 6 bits
> unused */
> #define V4L2_PIX_FMT_IPU3_SBGGR10v4l2_fourcc('i', 'p', '3', 'b') /* IPU3
> packed 10-bit BGGR bayer */
>
Regards,
Hans
Re: [PATCH v2] [RFC v2] v4l2: add support for colorspace conversion for video capture
Hi Philipp, It is much appreciated that this old RFC of mine is picked up again. I always wanted to get this in, but I never had a driver where it would make sense to do so. On 09/05/2018 07:09 PM, Philipp Zabel wrote: > For video capture it is the driver that reports the colorspace, add: "transfer function," > Y'CbCr/HSV encoding and quantization range used by the video, and there > is no way to request something different, even though many HDTV > receivers have some sort of colorspace conversion capabilities. > > For output video this feature already exists since the application > specifies this information for the video format it will send out, and > the transmitter will enable any available CSC if a format conversion has > to be performed in order to match the capabilities of the sink. > > For video capture we propose adding new pix_format flags: > V4L2_PIX_FMT_FLAG_CSC_COLORSPACE, V4L2_PIX_FMT_FLAG_CSC_YCBCR_ENC, > V4L2_PIX_FMT_FLAG_CSC_HSV_ENC, V4L2_PIX_FMT_FLAG_CSC_QUANTIZATION, and > V4L2_PIX_FMT_FLAG_CSC_XFER_FUNC. These are set by the driver to indicate > its conversion features. When set by the application, the driver will > interpret the colorspace, ycbcr_enc/hsv_enc, quantization and xfer_func > fields as the requested colorspace information and will attempt to do > the conversion it supports. > > Drivers do not have to actually look at the flags: if the flags are not > set, then the colorspace, ycbcr_enc and quantization fields are set to > the default values by the core, i.e. just pass on the received format > without conversion. Thinking about this some more, I don't think this is quite the right approach. Having userspace set these flags with S_FMT if they want to do explicit conversions makes sense, and that part we can keep. But to signal the capabilities I think should be done via new flags for VIDIOC_ENUM_FMT. Basically the same set of flags, but for the flags field of struct v4l2_fmtdesc. One thing that's not clear to me is what happens if userspace sets one or more flags and calls S_FMT for a driver that doesn't support this. Are the flags zeroed in that case upon return? I don't think so, but I think that is already true for the existing flag V4L2_PIX_FMT_FLAG_PREMUL_ALPHA. I wonder if V4L2_PIX_FMT_FLAG_PREMUL_ALPHA should also get an equivalent flag for v4l2_fmtdesc. Then we can just document that v4l2_format flags are only valid if they are also defined in v4l2_fmtdesc. Does anyone have better ideas for this? Regards, Hans > > Signed-off-by: Hans Verkuil > Signed-off-by: Philipp Zabel > --- > Changes since v1 [1] > - convert to rst > - split V4L2_PIX_FMT_FLAG_REQUEST_CSC into four separate flags for >colorspace, ycbcr_enc/hsv_enc, quantization, and xfer_func > - let driver set flags to indicate supported features > > [1] https://patchwork.linuxtv.org/patch/28847/ > --- > .../media/uapi/v4l/pixfmt-reserved.rst| 41 +++ > .../media/uapi/v4l/pixfmt-v4l2-mplane.rst | 16 ++-- > Documentation/media/uapi/v4l/pixfmt-v4l2.rst | 37 ++--- > drivers/media/v4l2-core/v4l2-ioctl.c | 12 ++ > include/uapi/linux/videodev2.h| 5 +++ > 5 files changed, 94 insertions(+), 17 deletions(-) > > diff --git a/Documentation/media/uapi/v4l/pixfmt-reserved.rst > b/Documentation/media/uapi/v4l/pixfmt-reserved.rst > index 38af1472a4b4..c1090027626c 100644 > --- a/Documentation/media/uapi/v4l/pixfmt-reserved.rst > +++ b/Documentation/media/uapi/v4l/pixfmt-reserved.rst > @@ -261,3 +261,44 @@ please make a proposal on the linux-media mailing list. > by RGBA values (128, 192, 255, 128), the same pixel described with > premultiplied colors would be described by RGBA values (64, 96, > 128, 128) > +* - ``V4L2_PIX_FMT_FLAG_CSC_COLORSPACE`` > + - 0x0002 > + - Set by the driver to indicate colorspace conversion support. Set by > the > + application to request conversion to the specified colorspace. It is > + only used for capture and is ignored for output streams. If set by the > + application, then request the driver to do colorspace conversion from > + the received colorspace to the requested colorspace by setting the > + ``colorspace`` field of struct :c:type:`v4l2_pix_format`. > +* - ``V4L2_PIX_FMT_FLAG_CSC_YCBCR_ENC`` > + - 0x0004 > + - Set by the driver to indicate Y'CbCr encoding conversion support. Set > + by the application to request conversion to the specified Y'CbCr > + encoding. It is only used for capture and is ignored for output > + streams. If set by the application, then request the driver to convert > + from the received Y'CbCr encoding to the requested encoding by setting > + the ``ycbcr_enc`` field of struct :c:type:`v4l2_pix_format`. > +* - ``V4L2_PIX_FMT_FLAG_CSC_HSV_ENC`` > + - 0x0004 > + - Set by the driver to indicate HSV encoding conversion support. Set > + by the a
[PATCH v3 4/4] MAINTAINERS: add entry for i.MX PXP media mem2mem driver
Signed-off-by: Philipp Zabel --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a5b256b25905..2e23c644f5d3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8994,6 +8994,13 @@ F: drivers/staging/media/imx/ F: include/linux/imx-media.h F: include/media/imx.h +MEDIA DRIVER FOR FREESCALE IMX PXP +M: Philipp Zabel +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +S: Maintained +F: drivers/media/platform/imx-pxp.[ch] + MEDIA DRIVERS FOR HELENE M: Abylay Ospan L: linux-media@vger.kernel.org -- 2.18.0
[PATCH v3 2/4] ARM: dts: imx6ull: add pxp support
Add the device node for the i.MX6ULL Pixel Pipeline (PXP).
Signed-off-by: Philipp Zabel
---
No changes since v2.
---
arch/arm/boot/dts/imx6ul.dtsi | 8
arch/arm/boot/dts/imx6ull.dtsi | 6 ++
2 files changed, 14 insertions(+)
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 6dc0b569acdf..051d42676160 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -945,6 +945,14 @@
status = "disabled";
};
+ pxp: pxp@21cc000 {
+ compatible = "fsl,imx6ul-pxp";
+ reg = <0x021cc000 0x4000>;
+ interrupts = ;
+ clock-names = "axi";
+ clocks = <&clks IMX6UL_CLK_PXP>;
+ };
+
qspi: qspi@21e {
#address-cells = <1>;
#size-cells = <0>;
diff --git a/arch/arm/boot/dts/imx6ull.dtsi b/arch/arm/boot/dts/imx6ull.dtsi
index cd1776a7015a..c0518490b58c 100644
--- a/arch/arm/boot/dts/imx6ull.dtsi
+++ b/arch/arm/boot/dts/imx6ull.dtsi
@@ -57,3 +57,9 @@
};
};
};
+
+&pxp {
+ compatible = "fsl,imx6ull-pxp";
+ interrupts = ,
+;
+};
--
2.18.0
[PATCH v3 3/4] media: imx-pxp: add i.MX Pixel Pipeline driver
Add a V4L2 mem-to-mem scaler/CSC driver for the Pixel Pipeline (PXP)
version found on i.MX6ULL SoCs. A similar variant is used on i.MX7D.
Since this driver only uses the legacy pipeline, it should be reasonably
easy to extend it to work with the older PXP versions found on i.MX6UL,
i.MX6SX, i.MX6SL, i.MX28, and i.MX23.
The driver supports scaling and colorspace conversion. There is
currently no support for rotation, alpha-blending, and the LUTs.
Signed-off-by: Philipp Zabel
---
Changes since v2:
- fix Kconfig whitespace
- remove unused defines
- fix video_unregister_device/v4l2_m2m_release order in pxp_remove
- use GPL-2.0+ instead of GPL-2.0-or-later SPDX license identifer
- remove pxp_default_ycbcr_enc/quantization, always map to default
encoding/quantization
- rename pxp_fixup_colorimetry to pxp_fixup_colorimetry_cap
---
drivers/media/platform/Kconfig |9 +
drivers/media/platform/Makefile |2 +
drivers/media/platform/imx-pxp.c | 1752 ++
drivers/media/platform/imx-pxp.h | 1685
4 files changed, 3448 insertions(+)
create mode 100644 drivers/media/platform/imx-pxp.c
create mode 100644 drivers/media/platform/imx-pxp.h
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 94c1fe0e9787..f6275874ec9e 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -181,6 +181,15 @@ config VIDEO_CODA
config VIDEO_IMX_VDOA
def_tristate VIDEO_CODA if SOC_IMX6Q || COMPILE_TEST
+config VIDEO_IMX_PXP
+ tristate "i.MX Pixel Pipeline (PXP)"
+ depends on VIDEO_DEV && VIDEO_V4L2 && (ARCH_MXC || COMPILE_TEST)
+ select VIDEOBUF2_DMA_CONTIG
+ select V4L2_MEM2MEM_DEV
+ help
+ The i.MX Pixel Pipeline is a memory-to-memory engine for scaling,
+ color space conversion, and rotation.
+
config VIDEO_MEDIATEK_JPEG
tristate "Mediatek JPEG Codec driver"
depends on MTK_IOMMU_V1 || COMPILE_TEST
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 41322ab65802..6ab6200dd9c9 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -25,6 +25,8 @@ obj-$(CONFIG_VIDEO_TI_CAL)+= ti-vpe/
obj-$(CONFIG_VIDEO_MX2_EMMAPRP)+= mx2_emmaprp.o
obj-$(CONFIG_VIDEO_CODA) += coda/
+obj-$(CONFIG_VIDEO_IMX_PXP)+= imx-pxp.o
+
obj-$(CONFIG_VIDEO_SH_VEU) += sh_veu.o
obj-$(CONFIG_CEC_GPIO) += cec-gpio/
diff --git a/drivers/media/platform/imx-pxp.c b/drivers/media/platform/imx-pxp.c
new file mode 100644
index ..68ecfed7098b
--- /dev/null
+++ b/drivers/media/platform/imx-pxp.c
@@ -0,0 +1,1752 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * i.MX Pixel Pipeline (PXP) mem-to-mem scaler/CSC/rotator driver
+ *
+ * Copyright (c) 2018 Pengutronix, Philipp Zabel
+ *
+ * based on vim2m
+ *
+ * Copyright (c) 2009-2010 Samsung Electronics Co., Ltd.
+ * Pawel Osciak,
+ * Marek Szyprowski,
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "imx-pxp.h"
+
+static unsigned int debug;
+module_param(debug, uint, 0644);
+MODULE_PARM_DESC(debug, "activates debug info");
+
+#define MIN_W 8
+#define MIN_H 8
+#define MAX_W 4096
+#define MAX_H 4096
+#define ALIGN_W 3 /* 8x8 pixel blocks */
+#define ALIGN_H 3
+
+/* Flags that indicate a format can be used for capture/output */
+#define MEM2MEM_CAPTURE(1 << 0)
+#define MEM2MEM_OUTPUT (1 << 1)
+
+#define MEM2MEM_NAME "pxp"
+
+/* Flags that indicate processing mode */
+#define MEM2MEM_HFLIP (1 << 0)
+#define MEM2MEM_VFLIP (1 << 1)
+
+#define dprintk(dev, fmt, arg...) \
+ v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg)
+
+struct pxp_fmt {
+ u32 fourcc;
+ int depth;
+ /* Types the format can be used for */
+ u32 types;
+};
+
+static struct pxp_fmt formats[] = {
+ {
+ .fourcc = V4L2_PIX_FMT_XBGR32,
+ .depth = 32,
+ /* Both capture and output format */
+ .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT,
+ }, {
+ .fourcc = V4L2_PIX_FMT_ABGR32,
+ .depth = 32,
+ /* Capture-only format */
+ .types = MEM2MEM_CAPTURE,
+ }, {
+ .fourcc = V4L2_PIX_FMT_BGR24,
+ .depth = 24,
+ .types = MEM2MEM_CAPTURE,
+ }, {
+ .fourcc = V4L2_PIX_FMT_RGB565,
+ .depth = 16,
+ .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT,
+ }, {
+ .fourcc = V4L2_PIX_FMT_RGB555,
+ .depth = 16,
+ .types = MEM2MEM_CAPTURE | MEM2MEM_OUTPUT,
+ }, {
+ .fourcc = V4L2_PIX_FMT_RGB444,
+ .depth = 16,
+
[PATCH v3 1/4] dt-bindings: media: Add i.MX Pixel Pipeline binding
Add DT binding documentation for the Pixel Pipeline (PXP) found on
various NXP i.MX SoCs.
Signed-off-by: Philipp Zabel
Reviewed-by: Rob Herring
---
No changes since v2.
---
.../devicetree/bindings/media/fsl-pxp.txt | 26 +++
1 file changed, 26 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt
diff --git a/Documentation/devicetree/bindings/media/fsl-pxp.txt
b/Documentation/devicetree/bindings/media/fsl-pxp.txt
new file mode 100644
index ..2477e7f87381
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/fsl-pxp.txt
@@ -0,0 +1,26 @@
+Freescale Pixel Pipeline
+
+
+The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine
+that supports scaling, colorspace conversion, alpha blending, rotation, and
+pixel conversion via lookup table. Different versions are present on various
+i.MX SoCs from i.MX23 to i.MX7.
+
+Required properties:
+- compatible: should be "fsl,-pxp", where SoC can be one of imx23, imx28,
+ imx6dl, imx6sl, imx6ul, imx6sx, imx6ull, or imx7d.
+- reg: the register base and size for the device registers
+- interrupts: the PXP interrupt, two interrupts for imx6ull and imx7d.
+- clock-names: should be "axi"
+- clocks: the PXP AXI clock
+
+Example:
+
+pxp@21cc000 {
+ compatible = "fsl,imx6ull-pxp";
+ reg = <0x021cc000 0x4000>;
+ interrupts = ,
+;
+ clock-names = "axi";
+ clocks = <&clks IMX6UL_CLK_PXP>;
+};
--
2.18.0
[PATCH v3 0/4] i.MX PXP scaler/CSC driver
The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine that supports scaling, colorspace conversion, alpha blending, rotation, and pixel conversion via lookup table. Different versions are present on various i.MX SoCs from i.MX23 to i.MX7. The latest versions on i.MX6ULL and i.MX7D have grown an additional pipeline for dithering and e-ink update processing that is ignored by this driver. This series adds a V4L2 mem-to-mem scaler/CSC driver for the PXP version found on i.MX6ULL SoCs which is a size reduced variant of the i.MX7 PXP. The driver uses only the legacy pipeline, so it should be reasonably easy to extend it to work with the older PXP versions found on i.MX6UL, i.MX6SX, i.MX6SL, i.MX28, and i.MX23. The driver supports scaling and colorspace conversion. There is currently no support for rotation, alpha-blending, and the LUTs. Changes since v2: - fix Kconfig whitespace - remove unused defines - fix video_unregister_device/v4l2_m2m_release order in pxp_remove - use GPL-2.0+ instead of GPL-2.0-or-later SPDX license identifer - remove pxp_default_ycbcr_enc/quantization, always map to default encoding/quantization - rename pxp_fixup_colorimetry to pxp_fixup_colorimetry_cap regards Philipp Philipp Zabel (4): dt-bindings: media: Add i.MX Pixel Pipeline binding ARM: dts: imx6ull: add pxp support media: imx-pxp: add i.MX Pixel Pipeline driver MAINTAINERS: add entry for i.MX PXP media mem2mem driver .../devicetree/bindings/media/fsl-pxp.txt | 26 + MAINTAINERS |7 + arch/arm/boot/dts/imx6ul.dtsi |8 + arch/arm/boot/dts/imx6ull.dtsi|6 + drivers/media/platform/Kconfig|9 + drivers/media/platform/Makefile |2 + drivers/media/platform/imx-pxp.c | 1752 + drivers/media/platform/imx-pxp.h | 1685 8 files changed, 3495 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt create mode 100644 drivers/media/platform/imx-pxp.c create mode 100644 drivers/media/platform/imx-pxp.h -- 2.18.0
Re: [PATCH v2 1/4] dt-bindings: media: Add i.MX Pixel Pipeline binding
Hi Stefan, thank you for your comments. On Wed, 2018-09-05 at 19:10 +0200, Stefan Wahren wrote: > Hi Philipp, > > > Philipp Zabel hat am 5. September 2018 um 12:00 > > geschrieben: > > > > > > Add DT binding documentation for the Pixel Pipeline (PXP) found on > > various NXP i.MX SoCs. > > > > Signed-off-by: Philipp Zabel > > Reviewed-by: Rob Herring > > --- > > .../devicetree/bindings/media/fsl-pxp.txt | 26 +++ > > 1 file changed, 26 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt > > > > diff --git a/Documentation/devicetree/bindings/media/fsl-pxp.txt > > b/Documentation/devicetree/bindings/media/fsl-pxp.txt > > new file mode 100644 > > index ..2477e7f87381 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/fsl-pxp.txt > > @@ -0,0 +1,26 @@ > > +Freescale Pixel Pipeline > > + > > + > > +The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine > > +that supports scaling, colorspace conversion, alpha blending, rotation, and > > +pixel conversion via lookup table. Different versions are present on > > various > > +i.MX SoCs from i.MX23 to i.MX7. > > + > > +Required properties: > > +- compatible: should be "fsl,-pxp", where SoC can be one of imx23, > > imx28, > > + imx6dl, imx6sl, imx6ul, imx6sx, imx6ull, or imx7d. > > please correct me if i'm wrong, but the driver in patch #3 only > support imx6ull That is correct. I assume it should work on i.MX7D mostly unchanged, by just adding a compatible. The others probably require some register layout changes. > so this binding is misleading. I disagree. The binding document specifies how PXP hardware should be described in the device tree. It should be seen completely separate from any driver implementation. There is no reason to leave out SoCs that are known to contain the PXP from this description just because some driver doesn't implement support for them. Similarly, there is no reason to remove the second interrupt just because the current Linux driver doesn't use it. > As a user i would expect that binding and driver are in sync. This expectation is at odds with the purpose of DT bindings, which is to describe the hardware, not to document driver features. (Which driver, anyway? Drivers for other operating systems or bootloaders could have a different set of supported SoCs and features). regards Philipp
Re: [PATCH v2 3/4] media: imx-pxp: add i.MX Pixel Pipeline driver
On Thu, 2018-09-06 at 09:57 +0200, Hans Verkuil wrote: [...] > > If userspace sets xfer_func explicitly, it will get the explicit default > > ycbcr_enc and quantization values. > > > > I think I did this to make v4l2-compliance at some point, but it could > > be that the explicit output->capture colorimetry copy for RGB->RGB and > > YUV->YUV conversions has me covered now. > > This xfer_func test makes no sense. xfer_func is completely ignored by the > driver (other than copying it from output to capture queue) since it can't > make any changes to it anyway. > > What you are trying to do in pxp_fixup_colorimetry() is to figure out the > ycbcr_enc and quantization values for the capture queue. Yes. I checked again without that. Since there is the forced out->cap copy for RGB->RGB and YUV->YUV conversions in pxp_fixup_colorimetry, v4l2-compliance is happy anyway. The pxp_default_quant/ycbcr_enc functions are removed now. > BTW, can you rename pxp_fixup_colorimetry to pxp_fixup_colorimetry_cap or > something? Since it is specifically for the capture queue. Ok. > These values depend entirely on the capture queue pixelformat and on the > colorspace and not on the xfer_func value. > > So just do: > > bool is_rgb = !pxp_v4l2_pix_fmt_is_yuv(dst_fourcc); > *ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(ctx->colorspace); > *quantization = V4L2_MAP_QUANTIZATION_DEFAULT(is_rgb, ctx->colorspace, > *ycbcr_enc); That's almost exactly what I ended up with, thank you. > BTW, I just noticed that the V4L2_MAP_QUANTIZATION_DEFAULT macro no longer > uses ycbcr_enc. The comment in videodev2.h should be updated. I can't > change the define as it is used in applications (and we might need to > depend on it again in the future anyway). > > If this code will give you v4l2-compliance issues, please let me know. > It shouldn't AFAICT. There are no complaints anymore. regards Philipp
Re: [PATCH v3 1/2] media: ov5640: Re-work MIPI startup sequence
Hello Loic,
On Thu, Sep 06, 2018 at 10:13:53AM +0200, Loic Poulain wrote:
> On 6 September 2018 at 09:48, jacopo mondi wrote:
> > Hello Loic,
> >thanks for looking into this
> >
> > On Tue, Sep 04, 2018 at 07:22:50PM +0200, Loic Poulain wrote:
> >> Hi Jacopo,
> >>
> >> > - 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
> >>
> >> Does 2-Lanes work with this config?
> >> AFAIU, if 2-Lanes is bit 5, value should be 0x25 and 0x20.
> >>
> >
> > Yes, confusing.
> >
> > The sensor manual reports
> > 0x300e[7:5] = 000 one lane mode
> > 0x300e[7:5] = 001 two lanes mode
> >
> > Although this configuration works with 2 lanes, and the application
> > note I have, with the suggested settings for MIPI CSI-2 2 lanes
> > reports 0x40 to be the 2 lanes mode...
> >
> > I used that one, also because the removed entry from the settings blob
> > is:
> > - {0x300e, 0x45, 0, 0}, {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
> > + {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
> >
> > So it was using BIT(6) already.
>
> Yes, it was setting BIT(6) from static config and BIT(5) from the
> ov5640_set_stream_mipi function. In your patch you don't set
> BIT(5) anymore.
>
> So it's not clear to me why it is still working, and the datasheet does
> not help a lot on this (BIT(6) is for debug modes).
> FYI I tried with BIT(5) only but it does not work (though I did not
> investigate a lot).
Thanks. Is your setup using 1 or 2 lanes? (I assume 2...)
Another question, unrelated to this specific issue: was the ov5640
working with dragonboard before this patch? I'm asking as I've seen
different behaviors between different platforms, and knowing this
fixes a widespread one like dragonboard is, would help getting this
patches in faster :)
Thanks
j
>
> > Anyway, it works for me with 2 lanes (and I assume Steve), you have tested
> > too, with how many lanes are you working with?
> >
> > Anyway, a comment there might be nice to have... Will add in next
> > version
>
> Definitely.
>
> Regards,
> Loic
signature.asc
Description: PGP signature
Re: [PATCH v3 1/2] media: ov5640: Re-work MIPI startup sequence
On 6 September 2018 at 09:48, jacopo mondi wrote:
> Hello Loic,
>thanks for looking into this
>
> On Tue, Sep 04, 2018 at 07:22:50PM +0200, Loic Poulain wrote:
>> Hi Jacopo,
>>
>> > - 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
>>
>> Does 2-Lanes work with this config?
>> AFAIU, if 2-Lanes is bit 5, value should be 0x25 and 0x20.
>>
>
> Yes, confusing.
>
> The sensor manual reports
> 0x300e[7:5] = 000 one lane mode
> 0x300e[7:5] = 001 two lanes mode
>
> Although this configuration works with 2 lanes, and the application
> note I have, with the suggested settings for MIPI CSI-2 2 lanes
> reports 0x40 to be the 2 lanes mode...
>
> I used that one, also because the removed entry from the settings blob
> is:
> - {0x300e, 0x45, 0, 0}, {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
> + {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
>
> So it was using BIT(6) already.
Yes, it was setting BIT(6) from static config and BIT(5) from the
ov5640_set_stream_mipi function. In your patch you don't set
BIT(5) anymore.
So it's not clear to me why it is still working, and the datasheet does
not help a lot on this (BIT(6) is for debug modes).
FYI I tried with BIT(5) only but it does not work (though I did not
investigate a lot).
> Anyway, it works for me with 2 lanes (and I assume Steve), you have tested
> too, with how many lanes are you working with?
>
> Anyway, a comment there might be nice to have... Will add in next
> version
Definitely.
Regards,
Loic
Re: [PATCH v2 3/4] media: imx-pxp: add i.MX Pixel Pipeline driver
On 09/05/2018 03:20 PM, Philipp Zabel wrote:
> On Wed, 2018-09-05 at 14:50 +0200, Hans Verkuil wrote:
>>> +static enum v4l2_ycbcr_encoding pxp_default_ycbcr_enc(struct pxp_ctx *ctx)
>>> +{
>>> + if (ctx->xfer_func)
>>> + return V4L2_MAP_YCBCR_ENC_DEFAULT(ctx->colorspace);
>>> + else
>>> + return V4L2_YCBCR_ENC_DEFAULT;
>>> +}
>>> +
>>> +static enum v4l2_quantization
>>> +pxp_default_quant(struct pxp_ctx *ctx, u32 pixelformat,
>>> + enum v4l2_ycbcr_encoding ycbcr_enc)
>>> +{
>>> + bool is_rgb = !pxp_v4l2_pix_fmt_is_yuv(pixelformat);
>>> +
>>> + if (ctx->xfer_func)
>>
>> Why check for xfer_func? (same question for the previous function)
>
> That way if userspace sets
> V4L2_XFER_FUNC_DEFAULT
> V4L2_YCBCR_ENC_DEFAULT
> V4L2_QUANTIZATION_DEFAULT
> on the output queue, it will get
> V4L2_XFER_FUNC_DEFAULT
> V4L2_YCBCR_ENC_DEFAULT
> V4L2_QUANTIZATION_DEFAULT
> on the capture queue.
>
> If userspace sets xfer_func explicitly, it will get the explicit default
> ycbcr_enc and quantization values.
>
> I think I did this to make v4l2-compliance at some point, but it could
> be that the explicit output->capture colorimetry copy for RGB->RGB and
> YUV->YUV conversions has me covered now.
This xfer_func test makes no sense. xfer_func is completely ignored by the
driver (other than copying it from output to capture queue) since it can't
make any changes to it anyway.
What you are trying to do in pxp_fixup_colorimetry() is to figure out the
ycbcr_enc and quantization values for the capture queue.
BTW, can you rename pxp_fixup_colorimetry to pxp_fixup_colorimetry_cap or
something? Since it is specifically for the capture queue.
These values depend entirely on the capture queue pixelformat and on the
colorspace and not on the xfer_func value.
So just do:
bool is_rgb = !pxp_v4l2_pix_fmt_is_yuv(dst_fourcc);
*ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(ctx->colorspace);
*quantization = V4L2_MAP_QUANTIZATION_DEFAULT(is_rgb, ctx->colorspace,
*ycbcr_enc);
BTW, I just noticed that the V4L2_MAP_QUANTIZATION_DEFAULT macro no longer
uses ycbcr_enc. The comment in videodev2.h should be updated. I can't
change the define as it is used in applications (and we might need to
depend on it again in the future anyway).
If this code will give you v4l2-compliance issues, please let me know.
It shouldn't AFAICT.
Regards,
Hans
Re: [PATCH v3 1/2] media: ov5640: Re-work MIPI startup sequence
Hello Loic,
thanks for looking into this
On Tue, Sep 04, 2018 at 07:22:50PM +0200, Loic Poulain wrote:
> Hi Jacopo,
>
> > - 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
>
> Does 2-Lanes work with this config?
> AFAIU, if 2-Lanes is bit 5, value should be 0x25 and 0x20.
>
Yes, confusing.
The sensor manual reports
0x300e[7:5] = 000 one lane mode
0x300e[7:5] = 001 two lanes mode
Although this configuration works with 2 lanes, and the application
note I have, with the suggested settings for MIPI CSI-2 2 lanes
reports 0x40 to be the 2 lanes mode...
I used that one, also because the removed entry from the settings blob
is:
- {0x300e, 0x45, 0, 0}, {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
+ {0x302e, 0x08, 0, 0}, {0x4300, 0x3f, 0, 0},
So it was using BIT(6) already.
I do not remember if I tested BIT(5) or not, it would be interesting
if someone using a 1-lane interface could try '000' and '001' maybe.
Anyway, it works for me with 2 lanes (and I assume Steve), you have tested
too, with how many lanes are you working with?
Anyway, a comment there might be nice to have... Will add in next
version
Thanks
j
> > +* [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);
>
> Regards,
> Loic
signature.asc
Description: PGP signature
[PATCH 0/2] [media] Confidence pixel-format for Intel RealSense cameras
From: Sergey Dorodnicov Define new fourcc describing depth sensor confidence data used in Intel RealSense cameras. Confidence information is stored as packed 4 bits per pixel single-plane image. The patches were tested on 4.18-rc2 and merged with media_tree/master. Sergey Dorodnicov (2): CNF4 fourcc for 4 bit-per-pixel packed confidence information CNF4 pixel format for media subsystem Documentation/media/uapi/v4l/depth-formats.rst | 1 + Documentation/media/uapi/v4l/pixfmt-cnf4.rst | 30 ++ drivers/media/usb/uvc/uvc_driver.c | 5 + drivers/media/usb/uvc/uvcvideo.h | 3 +++ drivers/media/v4l2-core/v4l2-ioctl.c | 1 + include/uapi/linux/videodev2.h | 1 + 6 files changed, 41 insertions(+) create mode 100644 Documentation/media/uapi/v4l/pixfmt-cnf4.rst -- 2.7.4
[PATCH 1/2] CNF4 fourcc for 4 bit-per-pixel packed confidence information
From: Sergey Dorodnicov
Adding new fourcc CNF4 for 4 bit-per-pixel packed confidence information
provided by Intel RealSense depth cameras. Every two consecutive pixels
are packed into a single byte (little-endian).
Signed-off-by: Sergey Dorodnicov
Signed-off-by: Evgeni Raikhel
---
Documentation/media/uapi/v4l/depth-formats.rst | 1 +
Documentation/media/uapi/v4l/pixfmt-cnf4.rst | 30 ++
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
include/uapi/linux/videodev2.h | 1 +
4 files changed, 33 insertions(+)
create mode 100644 Documentation/media/uapi/v4l/pixfmt-cnf4.rst
diff --git a/Documentation/media/uapi/v4l/depth-formats.rst
b/Documentation/media/uapi/v4l/depth-formats.rst
index d1641e9..9533348 100644
--- a/Documentation/media/uapi/v4l/depth-formats.rst
+++ b/Documentation/media/uapi/v4l/depth-formats.rst
@@ -14,3 +14,4 @@ Depth data provides distance to points, mapped onto the image
plane
pixfmt-inzi
pixfmt-z16
+pixfmt-cnf4
diff --git a/Documentation/media/uapi/v4l/pixfmt-cnf4.rst
b/Documentation/media/uapi/v4l/pixfmt-cnf4.rst
new file mode 100644
index 000..d24fc1a
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-cnf4.rst
@@ -0,0 +1,30 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-CNF4:
+
+**
+V4L2_PIX_FMT_CNF4 ('CNF4')
+**
+
+Sensor confidence information as a 4 bits per pixel packed array
+
+Description
+===
+
+Proprietary format used by Intel RealSense Depth cameras containing sensor
+confidence information in range 0-15 with 0 indicating that the sensor was
+unable to resolve any signal and 15 indicating maximum level of confidence for
+the specific sensor (actual error margins might change from sensor to sensor).
+
+Every two consecutive pixels are packed into a single byte (bit order is
+little-endian).
+
+**Bit-packed representation.**
+
+.. flat-table::
+:header-rows: 0
+:stub-columns: 0
+:widths: 64 64
+
+* - Y'\ :sub:`01[3:0]`\ (bits 3--0) Y'\ :sub:`00[3:0]`\ (bits 7--4)
+ - Y'\ :sub:`03[3:0]`\ (bits 3--0) Y'\ :sub:`02[3:0]`\ (bits 7--4)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 54afc9c..eff296d 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1189,6 +1189,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
case V4L2_PIX_FMT_Y12I: descr = "Interleaved 12-bit Greyscale";
break;
case V4L2_PIX_FMT_Z16: descr = "16-bit Depth"; break;
case V4L2_PIX_FMT_INZI: descr = "Planar 10:16 Greyscale Depth";
break;
+ case V4L2_PIX_FMT_CNF4: descr = "4-bit Confidence (Packed)";
break;
case V4L2_PIX_FMT_PAL8: descr = "8-bit Palette"; break;
case V4L2_PIX_FMT_UV8: descr = "8-bit Chrominance UV 4-4";
break;
case V4L2_PIX_FMT_YVU410: descr = "Planar YVU 4:1:0"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 5d1a368..a47f603 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -676,6 +676,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_Z16 v4l2_fourcc('Z', '1', '6', ' ') /* Depth data
16-bit */
#define V4L2_PIX_FMT_MT21Cv4l2_fourcc('M', 'T', '2', '1') /* Mediatek
compressed block mode */
#define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar
Greyscale 10-bit and Depth 16-bit */
+#define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit
packed confidence information */
/* 10bit raw bayer packed, 32 bytes for every 25 pixels, last LSB 6 bits
unused */
#define V4L2_PIX_FMT_IPU3_SBGGR10 v4l2_fourcc('i', 'p', '3', 'b') /* IPU3
packed 10-bit BGGR bayer */
--
2.7.4
[PATCH 2/2] CNF4 pixel format for media subsystem
From: Sergey Dorodnicov
Registering new GUID used by Intel RealSense depth cameras with fourcc
CNF4, encoding sensor confidence information for every pixel.
Signed-off-by: Sergey Dorodnicov
Signed-off-by: Evgeni Raikhel
---
drivers/media/usb/uvc/uvc_driver.c | 5 +
drivers/media/usb/uvc/uvcvideo.h | 3 +++
2 files changed, 8 insertions(+)
diff --git a/drivers/media/usb/uvc/uvc_driver.c
b/drivers/media/usb/uvc/uvc_driver.c
index d46dc43..c8d40a4 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -214,6 +214,11 @@ static struct uvc_format_desc uvc_fmts[] = {
.guid = UVC_GUID_FORMAT_INZI,
.fcc= V4L2_PIX_FMT_INZI,
},
+ {
+ .name = "Confidence 4-bit Packed (CNF4)",
+ .guid = UVC_GUID_FORMAT_CNF4,
+ .fcc= V4L2_PIX_FMT_CNF4,
+ },
};
/*
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index e5f5d84..779bab2 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -154,6 +154,9 @@
#define UVC_GUID_FORMAT_INVI \
{ 'I', 'N', 'V', 'I', 0xdb, 0x57, 0x49, 0x5e, \
0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
+#define UVC_GUID_FORMAT_CNF4 \
+ { 'C', ' ', ' ', ' ', 0x00, 0x00, 0x10, 0x00, \
+0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
#define UVC_GUID_FORMAT_D3DFMT_L8 \
{0x32, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, \
--
2.7.4
