cron job: media_tree daily build: OK

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

Results of the daily build of media_tree:

date:   Sat Sep  8 04:00:16 CEST 2018
media-tree git hash:d842a7cf938b6e0f8a1aa9f1aec0476c9a599310
media_build git hash:   ed1d887e2c18299383c7258615130197c8ce4946
v4l-utils git hash: d26e4941419b05fcb2b6708ee32aef367c2ec4af
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-3-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/Saturday.log

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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


Query on V4L2 interlaced "height"

2018-09-07 Thread Satish Nagireddy
Hi,

I am working on interlaced capture devices. There is some confusion in
handling height parameter between application and driver.

Capture device is able to produce 1920x1080i, where one field
(top/bottom) resolution is 1920x540.

Query 1:
What should be the height passed by application to driver to capture
1920x1080i resolution?
According to v4l2 specification:
https://www.kernel.org/doc/html/v4.16/media/uapi/v4l/pixfmt-v4l2.html

Image height in pixels. If field is one of V4L2_FIELD_TOP,
V4L2_FIELD_BOTTOM or V4L2_FIELD_ALTERNATE then height refers to

the number of lines in the field, otherwise it refers to the number of
lines in the frame (which is twice the field height for interlaced
formats).

Query 2:
I can think of 4 possible cases here:

i) If application calling VIDIOC_TRY_FMT with filed as
V4L2_FIELD_ALTERNATE and capture hardware supports same

ii) If application calling VIDIOC_TRY_FMT with filed as
V4L2_FIELD_NONE and capture hardware supports same

iii) If application calling VIDIOC_TRY_FMT with field as
V4L2_FIELD_NONE (progressive) and capture hardware supports
V4L2_FIELD_ALTERNATE

iv) If application calling VIDIOC_TRY_FMT with field as
V4L2_FIELD_ALTERNATE (progressive) and capture hardware supports
V4L2_FIELD_NONE

The first 2 cases are straightforward. What should be the driver
behavior for iii and iv ? Should it alter height passed by the
application accordingly?

I see some of the capture drivers are dividing height by 2 if field is
V4L2_FIELD_ALTERNATE. Is this the right behavior?

Regards,
Satish


Re: [RFP] Stateless Codec Userspace Support

2018-09-07 Thread Nicolas Dufresne
Le vendredi 07 septembre 2018 à 16:34 +0200, Hans Verkuil a écrit :
> Support for stateless codecs and Request API will hopefully be merged
> for
> 4.20, and the next step is to discuss how to organize the userspace
> support.
> 
> Hopefully by the time the media summit starts we'll have some better
> ideas
> of what we want in this area.
> 
> Some userspace support is available from bootlin for the cedrus
> driver:
> 
>   - v4l2-request-test, that has a bunch of sample frames for various
> codecs and will rely solely on the kernel request api (and DRM
> for
> the display part) to test and bringup a particular driver
> https://github.com/bootlin/v4l2-request-test
> 
>   - libva-v4l2-request, that is a libva implementation using the
> request API
> https://github.com/bootlin/libva-v4l2-request
> 
> But this is more geared towards testing and less a 'proper'
> implementation.

Considering that libva is largely supported across media players,
browsers and media frameworks, the VA Driver approach seems like the
most promising solution to get short term usage. This way, we can share
the userspace code across various codec and also across V4L2 and DRM
subsystems.

That being said, a lot of userspace will need modification. Indeed, VA
do expose some of the DRM details for zero-copy path (like DMABuf
exportation). We can emulate this support, or simply enhance VA with
it's own V4L2 specific bits. It's too early to tell, and also I'm not
deep enough into VA driver interface to be able to give guidelines.

Another thing that most userspace rely on is the presence of VPP
functions. I notice some assembly code that does detiling in that libva
driver, I bet this is related to not having enabled some sort of HW VPP
yet on the Allwinner SoC. Overall, this does not seems like a problem,
the m2m interface is well suited for that and a VA driver can make use
of that. What will be needed is a better way to figure-out what these
VPP can do, things like CSC, deinterlacing, scaling, rotation, etc.
Just like in any other library, we need to be able to announce which
"function" are supported.

Putting my GStreamer hat back, I'd very like to have a native support
for these stateless CODEC, as this would give a bit more flexibility,
but this isn't something that one can write in a day (specially if that
happens on my spare or r&d time). Though, I'm looking forward into this
in order to come up with a library, a bit like the existing GStreamer
bitstream parser library, that could handle reference picture
management and lost frame concealment (a currently missing feature in
gstreamer-vaapi).

I think that most straighforward place to add direct support (without
VA abstraction) would be FFMPEG. If I understood well, they already
share most of the decoder layer needed between their software decoder
and the VAAPI one.

One place that haven't been mentioned, but seems rather important,
would be Android. Implementing a generic OMX component for the Android
OMX stack would get quite some traction, as the CODEC integration work
would become very much a kernel work. Having that handy, would help
convince the vendors that the V4L2 framework is worth it. Making the
OMX stack in Android as vendor agnostic as possible is also helping
Android folks in eventually getting rid of OMX. OMX specification is
mostly abandoned, with no-one to review new extensions.

> 
> I don't know yet how much time to reserve for this discussion. It's a
> bit too early for that. I would expect an hour minimum, likely more.
> 
> Regards,
> 
>   Hans


signature.asc
Description: This is a digitally signed message part


vidioc-cropcap/g-crop.rst: fix confusing sentence

2018-09-07 Thread Hans Verkuil
The note that the text refers to is actually *below* the type description,
not above.

Fix this.

Signed-off-by: Hans Verkuil 
---
diff --git a/Documentation/media/uapi/v4l/vidioc-cropcap.rst 
b/Documentation/media/uapi/v4l/vidioc-cropcap.rst
index a65dbec6b20b..0a7b8287fd38 100644
--- a/Documentation/media/uapi/v4l/vidioc-cropcap.rst
+++ b/Documentation/media/uapi/v4l/vidioc-cropcap.rst
@@ -58,7 +58,7 @@ overlay devices.
   - Type of the data stream, set by the application. Only these types
are valid here: ``V4L2_BUF_TYPE_VIDEO_CAPTURE``, 
``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``,
``V4L2_BUF_TYPE_VIDEO_OUTPUT``, ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE`` 
and
-   ``V4L2_BUF_TYPE_VIDEO_OVERLAY``. See :c:type:`v4l2_buf_type` and the 
note above.
+   ``V4L2_BUF_TYPE_VIDEO_OVERLAY``. See :c:type:`v4l2_buf_type` and the 
note below.
 * - struct :ref:`v4l2_rect `
   - ``bounds``
   - Defines the window within capturing or output is possible, this
diff --git a/Documentation/media/uapi/v4l/vidioc-g-crop.rst 
b/Documentation/media/uapi/v4l/vidioc-g-crop.rst
index a6ed43ba9ca3..b95ba6743cbd 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-crop.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-crop.rst
@@ -84,7 +84,7 @@ When cropping is not supported then no parameters are changed 
and
   - Type of the data stream, set by the application. Only these types
are valid here: ``V4L2_BUF_TYPE_VIDEO_CAPTURE``, 
``V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE``,
``V4L2_BUF_TYPE_VIDEO_OUTPUT``, ``V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE`` 
and
-   ``V4L2_BUF_TYPE_VIDEO_OVERLAY``. See :c:type:`v4l2_buf_type` and the 
note above.
+   ``V4L2_BUF_TYPE_VIDEO_OVERLAY``. See :c:type:`v4l2_buf_type` and the 
note below.
 * - struct :c:type:`v4l2_rect`
   - ``c``
   - Cropping rectangle. The same co-ordinate system as for struct


Re: [RFP] Stateless Codec Userspace Support

2018-09-07 Thread Maxime Ripard
On Fri, Sep 07, 2018 at 04:34:45PM +0200, Hans Verkuil wrote:
> Support for stateless codecs and Request API will hopefully be merged for
> 4.20, and the next step is to discuss how to organize the userspace support.
> 
> Hopefully by the time the media summit starts we'll have some better ideas
> of what we want in this area.
> 
> Some userspace support is available from bootlin for the cedrus driver:
> 
>   - v4l2-request-test, that has a bunch of sample frames for various
> codecs and will rely solely on the kernel request api (and DRM for
> the display part) to test and bringup a particular driver
> https://github.com/bootlin/v4l2-request-test
> 
>   - libva-v4l2-request, that is a libva implementation using the
> request API
> https://github.com/bootlin/libva-v4l2-request
> 
> But this is more geared towards testing and less a 'proper' implementation.

While the first one is definitely a test tool, I wouldn't consider the
latter as such. We have Kodi and VLC running with it, and we started
to work on using gstreamer on top of it, so it's definitely something
I would consider for real world use cases.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature


[RFP] Stateless Codec Userspace Support

2018-09-07 Thread Hans Verkuil
Support for stateless codecs and Request API will hopefully be merged for
4.20, and the next step is to discuss how to organize the userspace support.

Hopefully by the time the media summit starts we'll have some better ideas
of what we want in this area.

Some userspace support is available from bootlin for the cedrus driver:

  - v4l2-request-test, that has a bunch of sample frames for various
codecs and will rely solely on the kernel request api (and DRM for
the display part) to test and bringup a particular driver
https://github.com/bootlin/v4l2-request-test

  - libva-v4l2-request, that is a libva implementation using the
request API
https://github.com/bootlin/libva-v4l2-request

But this is more geared towards testing and less a 'proper' implementation.

I don't know yet how much time to reserve for this discussion. It's a
bit too early for that. I would expect an hour minimum, likely more.

Regards,

Hans


Re: [PATCH v2 5/5] media: ov5640: fix restore of last mode set

2018-09-07 Thread Laurent Pinchart
Hello Hugues,

On Thursday, 16 August 2018 18:07:54 EEST Hugues FRUCHET wrote:
> On 08/16/2018 12:10 PM, jacopo mondi wrote:
> > On Mon, Aug 13, 2018 at 12:19:46PM +0200, Hugues Fruchet wrote:
> > 
> >> Mode setting depends on last mode set, in particular
> >> because of exposure calculation when downscale mode
> >> change between subsampling and scaling.
> >> At stream on the last mode was wrongly set to current mode,
> >> so no change was detected and exposure calculation
> >> was not made, fix this.
> > 
> > I actually see a different issue here...
> 
> Which problem do you have exactly, you got a VGA JPEG instead of a QVGA 
> YUYV ?
> 
> > The issue I see here depends on the format programmed through
> > set_fmt() never being applied when using the sensor with a media
> > controller equipped device (in this case an i.MX6 board) through
> > capture sessions, and the not properly calculated exposure you see may
> > be a consequence of this.
> > 
> > I'll try to write down what I see, with the help of some debug output.
> > 
> > - At probe time mode 640x460@30 is programmed:
> > 
> >[1.651216] ov5640_probe: Initial mode with id: 2
> > 
> > - I set the format on the sensor's pad and it gets not applied but
> >marked as pending as the sensor is powered off:
> > 
> >#media-ctl --set-v4l2 "'ov5640 2-003c':0[fmt:UYVY2X8/320x240
> >field:none]"
> > [   65.611983] ov5640_set_fmt: NEW mode with id: 1 - PENDING
> 
> So here sensor->current_mode is set to <1>;//QVGA
> and sensor->pending_mode_change is set to true;
> 
> > - I start streaming with yavta, and the sensor receives a power on;
> >this causes the 'initial' format to be re-programmed and the pending
> >change to be ignored:
> > 
> >#yavta -c10 -n4 -f YUYV -s $320x240  -F"../frame-#.yuv" /dev/video4
> >
> > [   69.395018] ov5640_set_power:1805 - on
> > [   69.431342] ov5640_restore_mode:1711
> > [   69.996882] ov5640_set_mode: Apply mode with id: 0
> > 
> >The 'ov5640_set_mode()' call from 'ov5640_restore_mode()' clears the
> >sensor->pending flag, discarding the newly requested format, for
> >this reason, at s_stream() time, the pending flag is not set
> >anymore.
> 
> OK but before clearing sensor->pending_mode_change, set_mode() is
> loading registers corresponding to sensor->current_mode:
> static int ov5640_set_mode(struct ov5640_dev *sensor,
>  const struct ov5640_mode_info *orig_mode)
> {
> ==>   const struct ov5640_mode_info *mode = sensor->current_mode;
> ...
>   ret = ov5640_set_mode_direct(sensor, mode, exposure);
> 
> => so mode <1> is expected to be set now, so I don't understand your trace:
> "> [   69.996882] ov5640_set_mode: Apply mode with id: 0"
> Which variable do you trace that shows "0" ?
> 
> > Are you using a media-controller system? I suspect in non-mc cases,
> > the set_fmt is applied through a single power_on/power_off session, not
> > causing the 'restore_mode()' issue. Is this the case for you or your
> > issue is differnt?
> > 
> > Edit:
> > Mita-san tried to address the issue of the output pixel format not
> > being restored when the image format was restored in
> > 19ad26f9e6e1 ("media: ov5640: add missing output pixel format setting")
> > 
> > I understand the issue he tried to fix, but shouldn't the pending
> > format (if any) be applied instead of the initial one unconditionally?
> 
> This is what does the ov5640_restore_mode(), set the current mode 
> (sensor->current_mode), that is done through this line:
>   /* now restore the last capture mode */
>   ret = ov5640_set_mode(sensor, &ov5640_mode_init_data);
> => note that the comment above is weird, in fact it is the "current" 
> mode that is set.
> => note also that the 2nd parameter is not the mode to be set but the 
> previously applied mode ! (ie loaded in ov5640 registers). This is used
> to decide if we have to go to the "set_mode_exposure_calc" or 
> "set_mode_direct".
> 
> the ov5640_restore_mode() also set the current pixel format 
> (sensor->fmt), that is done through this line:
>   return ov5640_set_framefmt(sensor, &sensor->fmt);
> ==> This is what have fixed Mita-san, this line was missing previously, 
> leading to "mode registers" being loaded but not the "pixel format 
> registers".

This seems overly complicated to me. Why do we have to set the mode at power 
on time at all, why can't we do it at stream on time only, and simplify all 
this logic ?

> PS: There are two other "set mode" related changes that are related to
> this:
> 1) 6949d864776e ("media: ov5640: do not change mode if format or
> frame interval is unchanged")
> => this is merged in media master, unfortunately I've introduced a 
> regression on "pixel format" side that I've fixed in this patchset :
> 2) https://www.mail-archive.com/linux-media@vger.kernel.org/msg134413.html
> Symptom was a noisy image when capturing QVGA YUV (in fact captured as 
> JPEG data).

[snip]

-- 
Regards,

Re: [PATCH 5/6] media: isp: fix a warning about a wrong struct initializer

2018-09-07 Thread Laurent Pinchart
Hi Mauro,

As maintainers should be held to the same level of obligations as developers, 
and to avoid demotivating reviewers, could you handle comments you receive 
before pushing your own patches to your tree ? There should be no maintainer 
privilege here.

On Wednesday, 8 August 2018 18:45:49 EEST Laurent Pinchart wrote:
> Hi Mauro,
> 
> Thank you for the patch.
> 
> The subject line should be "media: omap3isp: ...".
> 
> On Wednesday, 8 August 2018 17:52:55 EEST Mauro Carvalho Chehab wrote:
> > As sparse complains:
> > drivers/media/platform/omap3isp/isp.c:303:39: warning: Using plain
> > integer
> > 
> > as NULL pointer
> > 
> > when a struct is initialized with { 0 }, actually the first
> > element of the struct is initialized with zeros, initializing the
> > other elements recursively. That can even generate gcc warnings
> > on nested structs.
> > 
> > So, instead, use the gcc-specific syntax for that (with is used
> > broadly inside the Kernel), initializing it with {};
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> > 
> >  drivers/media/platform/omap3isp/isp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/omap3isp/isp.c
> > b/drivers/media/platform/omap3isp/isp.c index 03354d513311..842e2235047d
> > 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> > @@ -300,7 +300,7 @@ static struct clk *isp_xclk_src_get(struct
> > of_phandle_args *clkspec, void *data) static int isp_xclk_init(struct
> > isp_device *isp)
> > 
> >  {
> >  
> > struct device_node *np = isp->dev->of_node;
> > 
> > -   struct clk_init_data init = { 0 };
> > +   struct clk_init_data init = {};
> 
> How about = { NULL }; to avoid a gcc-specific syntax ?
> 
> > unsigned int i;
> > 
> > for (i = 0; i < ARRAY_SIZE(isp->xclks); ++i)


-- 
Regards,

Laurent Pinchart





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

2018-09-07 Thread Loic Poulain
On 6 September 2018 at 10:48, jacopo mondi  wrote:
> 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 :)

I did not test without the patch, will do.


Re: [PATCH 2/2] media: ov5640: Fix auto-exposure disabling

2018-09-07 Thread jacopo mondi
HI Laurent,

On Thu, Sep 06, 2018 at 08:08:50PM +0300, Laurent Pinchart wrote:
> 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.

Thanks for the clarification.

Please note this series is superseded by Hugues' exposure and gain
fixes one, and my MIPI CSI-2 startup one (as it includes the timings
fix also sent there).

Thanks
   j
>
> > > 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_m

不上B2B也可以开发优质客户

2018-09-07 Thread h...@feibianli.cn
您好,
打扰您一分钟的时间

我司发软件是主动开发客户的一种方式,
把全球范围所有的意向客户给搜索出来,取得与他们联系,
快速成交一些订单的同时  还可以培养一批忠诚的合作伙伴。

软件主要功能及卖点:

  一、软件是和网络、搜索引擎同步更新,保证目标客户资源实时更新。

  二、一天可以联系上至少1个客户。效率高,目标精准。

  三、不限制关键词,您可以搜索任何产品,任何行业优质目标潜在客户信息。

  四、包含14个世界主流搜索引擎,4个黄页引擎,315个国家搜索引擎,
 并保持引擎持续更新中,可以多个搜索引擎同时搜索定位,效率极高。

  五、可以指定目标国家或区域单独精确定位目标潜在客户。

  六、支持多国语言全方位搜索定位。

详细了解 咨询Q 3309119269 在线QQ 根据 贵司产品 英文名称 演示操作了解 。