Re: [PATCH v5 0/9] Asynchronous UVC

[Pavel Machek]

On Tue 2018-11-06 21:27:11, Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> The Linux UVC driver has long provided adequate performance capabilities for
> web-cams and low data rate video devices in Linux while resolutions were low.
> 
> Modern USB cameras are now capable of high data rates thanks to USB3 with
> 1080p, and even 4k capture resolutions supported.
> 
> Cameras such as the Stereolabs ZED (bulk transfers) or the Logitech BRIO
> (isochronous transfers) can generate more data than an embedded ARM core is
> able to process on a single core, resulting in frame loss.
> 
> A large part of this performance impact is from the requirement to
> ‘memcpy’ frames out from URB packets to destination frames. This unfortunate
> requirement is due to the UVC protocol allowing a variable length header, and
> thus it is not possible to provide the target frame buffers directly.
> 
> Extra throughput is possible by moving the actual memcpy actions to a work
> queue, and moving the memcpy out of interrupt context thus allowing work tasks
> to be scheduled across multiple cores.

Hmm. Doing memcpy() on many cores is improvement but... not really.
Would it be possible to improve kernel<->user interface, so it says
"data is in this buffer, and it starts here" and so that memcpy in
kernel is not neccessary?
Pavel

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


signature.asc
Description: Digital signature

Re: [RFC PATCH v8 1/4] media: Media Device Allocator API

[Pavel Machek]

On Thu 2018-11-01 18:31:30, sh...@kernel.org wrote:
> From: Shuah Khan 
> 
> Media Device Allocator API to allows multiple drivers share a media device.
> Using this API, drivers can allocate a media device with the shared struct
> device as the key. Once the media device is allocated by a driver, other
> drivers can get a reference to it. The media device is released when all
> the references are released.

Sounds like a ... bad idea?

That's what new "media control" framework is for, no?

Why do you need this?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 2/5] v4l: controls: Add support for exponential bases, prefixes and units

[Pavel Machek]

Hi!

> Add support for exponential bases, prefixes as well as units for V4L2
> controls. This makes it possible to convey information on the relation
> between the control value and the hardware feature being controlled.
> 
> Signed-off-by: Sakari Ailus 

Sounds good.

> +/* V4L2 control exponential bases */
> +#define V4L2_CTRL_BASE_UNDEFINED 0
> +#define V4L2_CTRL_BASE_LINEAR1
> +#define V4L2_CTRL_BASE_2 2
> +#define V4L2_CTRL_BASE_1010

Do we also want to support logarithmic and 1/x? 

For example focus is usually in diopters and thats 1/meter...

> +/* V4L2 control units */
> +#define V4L2_CTRL_UNIT_UNDEFINED 0
> +#define V4L2_CTRL_UNIT_NONE  1
> +#define V4L2_CTRL_UNIT_SECOND2
> +#define V4L2_CTRL_UNIT_AMPERE3
> +#define V4L2_CTRL_UNIT_LINE  4
> +#define V4L2_CTRL_UNIT_PIXEL 5
> +#define V4L2_CTRL_UNIT_PIXELS_PER_SEC6
> +#define V4L2_CTRL_UNIT_HZ7

And Hz is 1/second.

Should we also have some support for ISO-sensitivity and f/ aperture numbers?


Thanks,
Pavel

Re: V4L2 analogue gain contol

[Pavel Machek]

Hi!

> I've been looking at various image sensor drivers to see how they expose
> gains, in particular analogue ones. What I found in 4.18 looks like a
> mess to me.
> 
> In particular, my interest is about separation of analogue vs digital
> gain and an understanding of what effect a change in gain has on the
> brightness of an image. The latter is characterized in the following
> table in the "linear" column.
> 
> driver  | CID | register name   | min | max  | def | linear | 
> comments
> +-+-+-+--+-++-
> adv7343 | G   | ADV7343_DAC2_OUTPUT_LEVEL   | -64 | 64   | 0   ||
> adv7393 | G   | ADV7393_DAC123_OUTPUT_LEVEL | -64 | 64   | 0   ||
> imx258  | A   | IMX258_REG_ANALOG_GAIN  | 0   | 8191 | 0   ||
> imx274  | G   | multiple| |  | | yes| [1]
> mt9m032 | G   | MT9M032_GAIN_ALL| 0   | 127  | 64  | no | [2]
> mt9m111 | G   | GLOBAL_GAIN | 0   | 252  | 32  | no | [3]
> mt9p031 | G   | MT9P031_GLOBAL_GAIN | 8   | 1024 | 8   | no | [4]
> mt9v011 | G   | multiple| 0   | 4063 | 32  ||
> mt9v032 | G   | MT9V032_ANALOG_GAIN | 16  | 64   | 16  | no | [5]
> ov13858 | A   | OV13858_REG_ANALOG_GAIN | 0   | 8191 | 128 ||
> ov2685  | A   | OV2685_REG_GAIN | 0   | 2047 | 54  ||
> ov5640  | G   | OV5640_REG_AEC_PK_REAL_GAIN | 0   | 1023 | 0   ||
> ov5670  | A   | OV5670_REG_ANALOG_GAIN  | 0   | 8191 | 128 ||
> ov5695  | A   | OV5695_REG_ANALOG_GAIN  | 16  | 248  | 248 ||
> mt9m001 | G   | MT9M001_GLOBAL_GAIN | 0   | 127  | 64  | no |
> mt9v022 | G   | MT9V022_ANALOG_GAIN | 0   | 127  | 64  ||
> 
> CID:
>   A -> V4L2_CID_ANALOGUE_GAIN
>   G -> V4L2_CID_GAIN, no V4L2_CID_ANALOGUE_GAIN present
> step: always 1
> comments:
> [1] controls a product of analogue and digital gain, value scales
> roughly linear
> [2] code comments contradict data sheet
> [3] it is not clear whether it also controls a digital gain.
> [4] controls a combination of analogue and digital gain
> [5] analogue only
> 
> The documentation (extended-controls.rst) says that the digital gain is
> supposed to be a linear fixed-point number with 0x100 meaning factor 1.
> The situation for analogue is much less precise.
> 
> Typically, the number of analogue gains is much smaller than the number
> of digital gains. No driver exposes more than 13 bit for the analogue
> gain and half of them use at most 8 bits.
> 
> Can we give more structure to the analogue gain as exposed by V4L2?
> Ideally, I'd like to query a driver for the possible gain values if
> there are few (say < 256) and their factors (which are often given in
> data sheets). The nature of gains though is that they are often similar
> to floating point numbers (2 ** exp * (1 + mant / precision)), which
> makes it difficult to represent them using min/max/step/default.

Yes, it would be nice to have uniform controls for that. And it would
be good if mapping to "ISO" sensitivity from digital photography existed.

> Would it be reasonable to add a new V4L2_CID_ANALOGUE_GAIN_MENU that
> claims linearity and uses fixed-point numbers like
> V4L2_CID_DIGITAL_GAIN? There already is the integer menu
> V4L2_CID_AUTO_EXPOSURE_BIAS, but it also affects the exposure.

I'm not sure if linear scale is really appropriate. You can expect
camera to do ISO100 or ISO200, but if your camera supports ISO48,
you don't really expect it to support ISO480100.

./drivers/media/i2c/et8ek8/et8ek8_driver.c already does that.

IOW logarithmic scale would be more appropriate; min/max would be
nice, and step 

> An important application is implementing a custom gain control when the
> built-in auto exposure is not applicable.

Looking at et8ek8 again, perhaps that's the right solution? Userland
just sets the gain, and the driver automatically selects best
analog/digital gain combination.

/*
 * This table describes what should be written to the sensor register
  * for each gain value. The gain(index in the table) is in terms of
   * 0.1EV, i.e. 10 indexes in the table give 2 time more gain [0] in
* the *analog gain, [1] in the digital gain
 *
  * Analog gain [dB] = 20*log10(regvalue/32); 0x20..0x100
   */
   

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


signature.asc
Description: Digital signature

Re: [PATCHv17 01/34] Documentation: v4l: document request API

[Pavel Machek]

Hi!

> Document the request API for V4L2 devices, and amend the documentation
> of system calls influenced by it.
> 
> Signed-off-by: Alexandre Courbot 
> Signed-off-by: Hans Verkuil 

Cc documentation people?

> +Synopsis
> +
> +
> +.. c:function:: int ioctl( int request_fd, MEDIA_REQUEST_IOC_QUEUE )
> +:name: MEDIA_REQUEST_IOC_QUEUE
> +
> +
> +Arguments
> +=
> +
> +``request_fd``
> +File descriptor returned by :ref:`MEDIA_IOC_REQUEST_ALLOC`.
> +
> +
> +Description
> +===
> +
> +If the media device supports :ref:`requests `, then
> +this request ioctl can be used to queue a previously allocated request.
> +
> +If the request was successfully queued, then the file descriptor can be
> +:ref:`polled ` to wait for the request to complete.

> +
> +If the request was already queued before, then ``EBUSY`` is returned.

I'd expect -1 to be returned and errno set to EBUSY?

> +
> +
> +On success 0 is returned, on error -1 and the ``errno`` variable is set
> +appropriately. The generic error codes are described at the
> +:ref:`Generic Error Codes ` chapter.
> +
> +EBUSY
> +The request was already queued.
> +EPERM
> +The application queued the first buffer directly, but later attempted
> +to use a request. It is not permitted to mix the two APIs.
> +ENOENT
> +The request did not contain any buffers. All requests are required
> +to have at least one buffer. This can also be returned if required
> +controls are missing.
> +ENOMEM
> +Out of memory when allocating internal data structures for this
> +request.
> +EINVAL
> +The request has invalid data.
> +EIO
> +The hardware is in a bad state. To recover, the application needs to
> +stop streaming to reset the hardware state and then try to restart
> +streaming.


Pavel

Re: [PATCHv16 01/34] Documentation: v4l: document request API

[Pavel Machek]

Hi!
> From: Alexandre Courbot 
> 
> Document the request API for V4L2 devices, and amend the documentation
> of system calls influenced by it.
> 
> Signed-off-by: Alexandre Courbot 
> Signed-off-by: Hans Verkuil 

> --- a/Documentation/media/uapi/v4l/buffer.rst
> +++ b/Documentation/media/uapi/v4l/buffer.rst
> @@ -306,10 +306,15 @@ struct v4l2_buffer
>- A place holder for future extensions. Drivers and applications
>   must set this to 0.
>  * - __u32
> -  - ``reserved``
> +  - ``request_fd``
>-
> -  - A place holder for future extensions. Drivers and applications
> - must set this to 0.
> +  - The file descriptor of the request to queue the buffer to. If 
> specified
> +and flag ``V4L2_BUF_FLAG_REQUEST_FD`` is set, then the buffer will be

Delete "specified and" -- 0 is valid fd?

> + queued to that request. This is set by the user when calling
> + :ref:`ioctl VIDIOC_QBUF ` and ignored by other ioctls.
> + If the device does not support requests, then ``EPERM`` will be 
> returned.
> + If requests are supported but an invalid request FD is given, then
> + ``ENOENT`` will be returned.

Should this still specify that this should be 0 if
V4L2_BUF_FLAG_REQUEST_FD is not set?

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


signature.asc
Description: Digital signature

Re: [PATCH v2 1/2] media: dt-bindings: bind nokia,n900-ir to generic pwm-ir-tx driver

[Pavel Machek]

Hi!

> Signed-off-by: Sean Young 
> ---
>  .../devicetree/bindings/media/nokia,n900-ir   | 20 ---
>  arch/arm/boot/dts/omap3-n900.dts  |  2 +-
>  drivers/media/rc/pwm-ir-tx.c  |  1 +
>  3 files changed, 2 insertions(+), 21 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/nokia,n900-ir
> 
> diff --git a/Documentation/devicetree/bindings/media/nokia,n900-ir 
> b/Documentation/devicetree/bindings/media/nokia,n900-ir
> deleted file mode 100644
> index 13a18ce37dd1..
> --- a/Documentation/devicetree/bindings/media/nokia,n900-ir
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -Device-Tree bindings for LIRC TX driver for Nokia N900(RX51)
> -
> -Required properties:
> - - compatible: should be "nokia,n900-ir".
> - - pwms: specifies PWM used for IR signal transmission.
> -
> -Example node:
> -
> - pwm9: dmtimer-pwm@9 {
> - compatible = "ti,omap-dmtimer-pwm";
> - ti,timers = <>;
> - ti,clock-source = <0x00>; /* timer_sys_ck */
> - #pwm-cells = <3>;
> - };
> -
> - ir: n900-ir {
> - compatible = "nokia,n900-ir";
> -
> - pwms = < 0 26316 0>; /* 38000 Hz */
> - };

Removing documentation is bad idea, I guess. The binding still exists
and new kernels should still support it.


> diff --git a/arch/arm/boot/dts/omap3-n900.dts 
> b/arch/arm/boot/dts/omap3-n900.dts
> index 182a53991c90..fd12dea15799 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -154,7 +154,7 @@
>   };
>  
>   ir: n900-ir {
> - compatible = "nokia,n900-ir";
> + compatible = "nokia,n900-ir", "pwm-ir-tx";
>   pwms = < 0 26316 0>; /* 38000 Hz */
>   };
>  

No problem.

> diff --git a/drivers/media/rc/pwm-ir-tx.c b/drivers/media/rc/pwm-ir-tx.c
> index 27d0f5837a76..272947b430c8 100644
> --- a/drivers/media/rc/pwm-ir-tx.c
> +++ b/drivers/media/rc/pwm-ir-tx.c
> @@ -30,6 +30,7 @@ struct pwm_ir {
>  };
>  
>  static const struct of_device_id pwm_ir_of_match[] = {
> + { .compatible = "nokia,n900-ir" },
>   { .compatible = "pwm-ir-tx", },
>   { },
>  };

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


signature.asc
Description: Digital signature

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

[Pavel Machek]

Hi!

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

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

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

Thanks,

Pavel

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


signature.asc
Description: Digital signature

Re: Software-only image processing for Intel "complex" cameras

[Pavel Machek]

Hi!

> > > > On Nokia N900, I have similar problems as Intel IPU3 hardware.
> > > >
> > > > Meeting notes say that pure software implementation is not fast
> > > > enough, but that it may be useful for debugging. It would be also
> > > > useful for me on N900, and probably useful for processing "raw" images
> > > > from digital cameras.
> > > >
> > > > There is sensor part, and memory-to-memory part, right? What is
> > > > the format of data from the sensor part? What operations would be
> > > > expensive on the CPU? If we did everthing on the CPU, what would be
> > > > maximum resolution where we could still manage it in real time?
> > >
> > > We can still use the memory-to-memory part (IMGU), even without 3A. It
> > > would just do demosaicing at default parameters and give us a YUV
> > > (NV12) frame. We could use some software component to analyze the YUV
> > > output and adjust sensor parameters accordingly. Possibly the part we
> > > already have in libv4l2 could just work?
> >
> > As soon as you get YUV, yes, libv4l2 should be able to work with that.
> >
> > OTOH using the memory-to-memory part is going to be tricky.
> 
> Why? I don't see any reason why it would be tricky. You just feed the
> right CAPTURE node with YUV buffers and the right OUTPUT node with raw
> buffers and that should work.

I have seen insides of libv4l2. I believe unpacking by hand will be
simpler / less tricky than passing buffers around.

Of course, long term both will be needed.

> > What
> > format is the data before demosaicing? Something common like BGGR10?
> 
> It's documented in detail in V4L2 documentation:
> > https://www.kernel.org/doc/html/latest/media/uapi/v4l/pixfmt-srggb10-ipu3.html

Ah, thanks for pointer. That's not too bad.

> > > The expensive operation would be analyzing the frame itself. I suppose
> > > you need to build some histogram representing brightness and white
> > > balance of the frame and then infer necessary sensor adjustments from
> > > that.
> >
> > That does not really have to be expensive. You can sample ... say
> > 1 pixels from the image, and get good-enough data for 3A.
> 
> Yes, but you need to do it relatively frequently to react for scene
> changes and also even 1 pixels (depending on distribution
> ) might mean fetching 1*cacheline bytes of data.

Yeah, there are tradeoffs...

> > > > Would it be possible to get access to machine with IPU3, or would
> > > > there be someone willing to test libv4l2 patches?
> > >
> > > I should be able to help with some basic testing, preferably limited
> > > to command line tools (but I might be able to create a test
> > > environment for X11 tools if really necessary).
> >
> > Could you just compile libv4l2 with sdlcam demo on the target, and
> > then ssh -X there from some sort of reasonable system?
> 
> Yes, that should work I guess. That would be a Chrome OS device (which
> doesn't include X11), so that would mean compiling some X11 libs (and
> probably some more dependencies) as well, but that's not impossible.

I believe you want to put Debian chroot in there... or get machine
with IPU3 where you can install Debian directly.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Software-only image processing for Intel "complex" cameras

[Pavel Machek]

Hi!

> > On Nokia N900, I have similar problems as Intel IPU3 hardware.
> >
> > Meeting notes say that pure software implementation is not fast
> > enough, but that it may be useful for debugging. It would be also
> > useful for me on N900, and probably useful for processing "raw" images
> > from digital cameras.
> >
> > There is sensor part, and memory-to-memory part, right? What is
> > the format of data from the sensor part? What operations would be
> > expensive on the CPU? If we did everthing on the CPU, what would be
> > maximum resolution where we could still manage it in real time?
> 
> We can still use the memory-to-memory part (IMGU), even without 3A. It
> would just do demosaicing at default parameters and give us a YUV
> (NV12) frame. We could use some software component to analyze the YUV
> output and adjust sensor parameters accordingly. Possibly the part we
> already have in libv4l2 could just work?

As soon as you get YUV, yes, libv4l2 should be able to work with that.

OTOH using the memory-to-memory part is going to be tricky. What
format is the data before demosaicing? Something common like BGGR10?

> The expensive operation would be analyzing the frame itself. I suppose
> you need to build some histogram representing brightness and white
> balance of the frame and then infer necessary sensor adjustments from
> that.

That does not really have to be expensive. You can sample ... say
1 pixels from the image, and get good-enough data for 3A.

> > Would it be possible to get access to machine with IPU3, or would
> > there be someone willing to test libv4l2 patches?
> 
> I should be able to help with some basic testing, preferably limited
> to command line tools (but I might be able to create a test
> environment for X11 tools if really necessary).

Could you just compile libv4l2 with sdlcam demo on the target, and
then ssh -X there from some sort of reasonable system?

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


signature.asc
Description: Digital signature

Re: [git:media_tree/master] media: i2c: lm3560: add support for lm3559 chip

[Pavel Machek]

Hi!

> This is an automatic generated email to let you know that the following patch 
> were queued:
> 
> Subject: media: i2c: lm3560: add support for lm3559 chip
> Author:  Pavel Machek 
> Date:Sun May 6 04:06:07 2018 -0400
> 
> Add support for LM3559, as found in Motorola Droid 4 phone, for
> example. SW interface seems to be identical.
> 
> Signed-off-by: Pavel Machek 
> Signed-off-by: Hans Verkuil 
> Signed-off-by: Mauro Carvalho Chehab 

I'd also recommend this one:

https://lkml.org/lkml/2018/5/6/46

Using most agressive settings by default is wrong.

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


signature.asc
Description: Digital signature

Re: Software-only image processing for Intel "complex" cameras

[Pavel Machek]

Hi!

> > > e. g. something like:
> > > 
> > >   board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
> > >   board_name = dmi_get_system_info(DMI_BOARD_NAME);
> > >   board_version = dmi_get_system_info(DMI_BOARD_NAME);
> > >   product_name = dmi_get_system_info(DMI_PRODUCT_NAME);
> > >   product_version = dmi_get_system_info(DMI_PRODUCT_VERSION);
> > > 
> > >   sprintf(dev->cap, "%s:%s:%s:%s", board_vendor, board_name,
> > > board_version, product_name, product_version);
> > > 
> > > (the real code should check if the values are filled, as not all BIOS 
> > > vendors use the
> > > same DMI fields)
> > > 
> > > With that, the library can auto-adjust without needing to run anything as
> > > root.
> > >   
> > Well actually most of those fields you're interested in are already exposed 
> > to userspace
> > through sysfs /sys/class/dmi/id/
> > 
> > Can't the library just pull them from there?
> 
> Good point. Yeah, the library could use them.

This could be done, but it would be better if libraries could query
neccessary information from v4l2 drivers.

If DMI was used, _library_ would need to know about new hardware,
which is  not desirable.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Software-only image processing for Intel "complex" cameras

[Pavel Machek]

Hi!

> > On Nokia N900, I have similar problems as Intel IPU3 hardware.
> > 
> > Meeting notes say that pure software implementation is not fast
> > enough, but that it may be useful for debugging. It would be also
> > useful for me on N900, and probably useful for processing "raw"
> > images
> > from digital cameras.
> > 
> > There is sensor part, and memory-to-memory part, right? What is
> > the format of data from the sensor part? What operations would be
> > expensive on the CPU? If we did everthing on the CPU, what would be
> > maximum resolution where we could still manage it in real time?
> 
> The IPU3 sensor produce a vendor specific form of bayer. If we manage
> to implement support for this format, it would likely be done in
> software. I don't think anyone can answer your other questions has no
> one have ever implemented this, hence measure performance.

I believe Intel has some estimates.

What is the maximum resolution of camera in the current Dell systems?

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


signature.asc
Description: Digital signature

Software-only image processing for Intel "complex" cameras

[Pavel Machek]

Hi!

On Nokia N900, I have similar problems as Intel IPU3 hardware.

Meeting notes say that pure software implementation is not fast
enough, but that it may be useful for debugging. It would be also
useful for me on N900, and probably useful for processing "raw" images
from digital cameras.

There is sensor part, and memory-to-memory part, right? What is
the format of data from the sensor part? What operations would be
expensive on the CPU? If we did everthing on the CPU, what would be
maximum resolution where we could still manage it in real time?

Would it be possible to get access to machine with IPU3, or would
there be someone willing to test libv4l2 patches?

Thanks and best regards,

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


signature.asc
Description: Digital signature

[PATCH, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> > We may do some magic to do v4l2_open_complex() in v4l2_open(), but I
> > believe that should be separate step.
> 
> In order to avoid breaking the ABI for existing apps, v4l2_open() should
> internally call v4l2_open_complex() (or whatever we call it at the new
> API design).

Ok. Here's updated patch. open_complex() now takes fd. Any other
issues?

Best regards,
Pavel

diff --git a/lib/include/libv4l2.h b/lib/include/libv4l2.h
index ea1870d..a0ec0a9 100644
--- a/lib/include/libv4l2.h
+++ b/lib/include/libv4l2.h
@@ -58,6 +58,10 @@ LIBV4L_PUBLIC extern FILE *v4l2_log_file;
invalid memory address will not lead to failure with errno being EFAULT,
as it would with a real ioctl, but will cause libv4l2 to break, and you
get to keep both pieces.
+
+   You can open complex pipelines by passing ".cv" file with pipeline
+   description to v4l2_open(). libv4l2 will open all the required
+   devices automatically in that case.
 */
 
 LIBV4L_PUBLIC int v4l2_open(const char *file, int oflag, ...);
diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h
index 1924c91..1ee697a 100644
--- a/lib/libv4l2/libv4l2-priv.h
+++ b/lib/libv4l2/libv4l2-priv.h
@@ -104,6 +104,7 @@ struct v4l2_dev_info {
void *plugin_library;
void *dev_ops_priv;
const struct libv4l_dev_ops *dev_ops;
+   struct v4l2_controls_map *map;
 };
 
 /* From v4l2-plugin.c */
@@ -130,4 +131,20 @@ static inline void v4l2_plugin_cleanup(void *plugin_lib, 
void *plugin_priv,
 extern const char *v4l2_ioctls[];
 void v4l2_log_ioctl(unsigned long int request, void *arg, int result);
 
+
+struct v4l2_control_map {
+   unsigned long control;
+   int fd;
+};
+
+struct v4l2_controls_map {
+   int main_fd;
+   int num_fds;
+   int num_controls;
+   struct v4l2_control_map map[];
+};
+
+int v4l2_open_pipeline(struct v4l2_controls_map *map, int v4l2_flags);
+LIBV4L_PUBLIC int v4l2_get_fd_for_control(int fd, unsigned long control);
+
 #endif
diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
index 2db25d1..ac430f0 100644
--- a/lib/libv4l2/libv4l2.c
+++ b/lib/libv4l2/libv4l2.c
@@ -70,6 +70,8 @@
 #include 
 #include 
 #include 
+#include 
+
 #include "libv4l2.h"
 #include "libv4l2-priv.h"
 #include "libv4l-plugin.h"
@@ -618,6 +620,8 @@ static void v4l2_update_fps(int index, struct 
v4l2_streamparm *parm)
devices[index].fps = 0;
 }
 
+static int v4l2_open_complex(int fd, int v4l2_flags);
+
 int v4l2_open(const char *file, int oflag, ...)
 {
int fd;
@@ -641,6 +645,21 @@ int v4l2_open(const char *file, int oflag, ...)
if (fd == -1)
return fd;
 
+   int len = strlen(file);
+   char *end = ".cv";
+   int len2 = strlen(end);
+   if ((len > len2) && (!strcmp(file + len - len2, end))) {
+   /* .cv extension */
+   struct stat sb;
+
+   if (fstat(fd, ) == 0) {
+   if ((sb.st_mode & S_IFMT) == S_IFREG) {
+   return v4l2_open_complex(fd, 0);
+   }
+   }
+   
+   }
+
if (v4l2_fd_open(fd, 0) == -1) {
int saved_err = errno;
 
@@ -787,6 +806,8 @@ no_capture:
if (index >= devices_used)
devices_used = index + 1;
 
+   devices[index].map = NULL;
+
/* Note we always tell v4lconvert to optimize src fmt selection for
   our default fps, the only exception is the app explicitly selecting
   a frame rate using the S_PARM ioctl after a S_FMT */
@@ -1056,12 +1077,47 @@ static int v4l2_s_fmt(int index, struct v4l2_format 
*dest_fmt)
return 0;
 }
 
+int v4l2_get_fd_for_control(int fd, unsigned long control)
+{
+   int index = v4l2_get_index(fd);
+   struct v4l2_controls_map *map;
+   int lo = 0;
+   int hi;
+
+   if (index < 0)
+   return fd;
+
+   map = devices[index].map;
+   if (!map)
+   return fd;
+   hi = map->num_controls;
+
+   while (lo < hi) {
+   int i = (lo + hi) / 2;
+   if (map->map[i].control == control) {
+   return map->map[i].fd;
+   }
+   if (map->map[i].control > control) {
+   hi = i;
+   continue;
+   }
+   if (map->map[i].control < control) {
+   lo = i+1;
+   continue;
+   }
+   printf("Bad: impossible condition in binary search\n");
+   exit(1);
+   }
+   return fd;
+}
+
 int v4l2_ioctl(int fd, unsigned long int request, ...)
 {
void *arg;
va_list ap;
int result, index, saved_err;
-   int is_capture_request = 0, stream_needs_locking = 0;
+   int is_capture_request = 0, stream_needs_locking = 0, 
+   is_subdev_request = 0;
 

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> I guess that could give some basic camera functionality on OMAP3-like 
> hardware.

Yeah, and that is the goal.

> For most of the current generation of imaging subsystems (e.g. Intel
> IPU3, Rockchip RKISP1) it's not enough. The reason is that there is
> more to be handled by userspace than just setting controls:
>  - configuring pixel formats, resolutions, crops, etc. through the
> whole pipeline - I guess that could be preconfigured per use case
> inside the configuration file, though,
>  - forwarding buffers between capture and processing pipelines, i.e.
> DQBUF raw frame from CSI2 video node and QBUF to ISP video node,
>  - handling metadata CAPTURE and OUTPUT buffers controlling the 3A
> feedback loop - this might be optional if all we need is just ability
> to capture some frames, but required for getting good quality,
>  - actually mapping legacy controls into the above metadata,

I just wanted to add few things:

It seems IPU3 and RKISP1 is really similar to what I have on
N900. Forwarding frames between parts of processing pipeline is not
neccessary, but the other parts are there.

There are also two points where you can gather the image data, either
(almost) raw GRBG10 data from the sensor, or scaled YUV data ready for
display. [And how to display that data without CPU involvement is
another, rather big, topic.]

Anyway, legacy applications expect simple webcams with bad pictures,
low resolution, and no AF support. And we should be able to provide
them with just that.

Best regards,

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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> > > > I don't think that kind of legacy apps is in use any more. I'd prefer
> > > > not to deal with them.  
> > > 
> > > In another thread ("[ANN v2] Complex Camera Workshop - Tokyo - Jun,
> > > 19"), Mauro has mentioned a number of those:
> > > 
> > > "open source ones (Camorama, Cheese, Xawtv, Firefox, Chromium, ...) and 
> > > closed
> > > source ones (Skype, Chrome, ...)"  
> > 
> > Yep, ok. Still would prefer not to deal with them.
> 
> I guess we'll end by needing to handle them. Anyway, now that PCs are
> starting to come with complex cameras[1], we'll need to address this
> with a focus on adding support for existing apps.
> 
> [1] we had a report from one specific model but I heard from a reliable
> source that there are already other devices with similar issues.

Well, now that the issue hit PCs, we'll get help from Linux distributors.

> > (Opening additional fds behind application's back is quite nasty,
> > those apps should really switch to v4l2_ variants).
> 
> Only closed source apps use the LD_PRELOAD hack. All the others
> use v4l2_ variants, but, as Nicolas mentioned at the other
> thread, there are a number of problems with the current approach.
> 
> Perhaps is time for us to not be limited to the current ABI, writing
> a new API from scratch, and then adding a compatibility layer to be
> used by apps that rely on v4l2_ variants, in order to avoid breaking
> the ABI and keep providing LD_PRELOAD. We can then convert the apps
> we use/care most to use the new ABI.

I believe we can support current features using existing libv4l2 abi
-- complex cameras should work as well as dumb ones do.

...something like that is certainly needed for testing.

But yes, long-term better interface is needed -- and code should not
be in library but in separate process.

> > Application ready to deal with additional fds being
> > opened. contrib/test/sdlcam will be the first one.
> > 
> > We may do some magic to do v4l2_open_complex() in v4l2_open(), but I
> > believe that should be separate step.
> 
> In order to avoid breaking the ABI for existing apps, v4l2_open() should
> internally call v4l2_open_complex() (or whatever we call it at the new
> API design).

Ok... everyone wants that. I can do that.

> > Ok, so... statistics support would be nice, but that is really
> > separate problem.
> > 
> > v4l2 already contains auto-exposure and auto-white-balance. I have
> > patches for auto-focus. But hardware statistics are not used.
> 
> Feel free to submit the auto-focus patches anytime. With all 3A
> algos there (even on a non-optimal way using hw stats), it will
> make easier for us when designing a solution that would work for
> both IMAP3 and ISP (and likely be generic enough for other hardware).

Let me finish the control propagation, first. My test hardware
supporting autofocus is N900, so I need control propagation to be able
to test autofocus.

> > Secondary use case is taking .jpg photos using sdlcam.
> 
> If a v4l2_complex_open() will, for now, be something that we don't
> export publicly, using it only for the tools already at libv4l2,
> I don't see much troubles on adding it, but I would hate to have to
> stick with this ABI. Otherwise, we should analyze it after having
> a bigger picture. So, better to wait for the Complex Camera
> Workshop before adding this.
> 
> Btw, would you be able to join us there (either locally or
> remotely via Google Hangouts)?

I don't plan going to Japan. Hangouts... I'm not big fan of Google,
but I'll try. I'm in GMT+2.

> > Test apps such as qv4l2 would be nice to have, and maybe I'll
> > experiment with capturing video somehow one day. I'm pretty sure it
> > will not be easy.
> 
> Capture video is a must have for PCs. The final solution should
> take it into account.

N900 is quite slow for JPEG compression (and I don't really want
solution depending on hardware extras like DSP)... so that will be
fun.

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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> > Thanks for thread pointer... I may be able to get in using hangouts.
> >
> > Anyway, there's big difference between open("/dev/video0") and
> > v4l2_open("/dev/video0"). I don't care about the first one, but yes we
> > should be able to support the second one eventually.
> >
> > And I don't think Mauro says apps like Camorama are of open() kind.
> 
> I don't think there is much difference between open() and v4l2_open(),
> since the former can be changed to the latter using LD_PRELOAD.

Well, if everyone thinks opening more than one fd in v4l2_open() is
okay, I can do that. Probably "if argument is regular file and has .mc
extension, use open_complex"?  

> If we simply add v4l2_open_complex() to libv4l, we would have to get
> developers of the applications (regardless of whether they use open()
> or v4l2_open()) to also support v4l2_open_complex(). For testing
> purposes of kernel developers it would work indeed, but I wonder if it
> goes anywhere beyond that.

I'd like people to think "is opening multiple fds okay at this
moment?" before switching to v4l2_open_complex(). But I guess I'm too careful.

> If all we need is some code to be able to test kernel camera drivers,
> I don't think there is any big problem in adding v4l2_open_complex()
> to libv4l. However, we must either ensure that either:
> a) it's not going to be widely used
> OR
> b) it is designed well enough to cover the complex cases I mentioned
> and which are likely to represent most of the hardware in the wild.

.mc descriptors should be indeed extensible enough for that.

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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

HI!

> > > Thanks for coming up with this proposal. Please see my comments below.
> > >
> > > > Ok, can I get any comments on this one?
> > > > v4l2_open_complex("/file/with/descriptor", 0) can be used to open
> > > > whole pipeline at once, and work if it as if it was one device.
> > >
> > > I'm not convinced if we should really be piggy backing on libv4l, but
> > > it's just a matter of where we put the code added by your patch, so
> > > let's put that aside.
> >
> > There was some talk about this before, and libv4l2 is what we came
> > with. Only libv4l2 is in position to propagate controls to right
> > devices.
> >
> > > Who would be calling this function?
> > >
> > > The scenario that I could think of is:
> > > - legacy app would call open(/dev/video?), which would be handled by
> > > libv4l open hook (v4l2_open()?),
> >
> > I don't think that kind of legacy apps is in use any more. I'd prefer
> > not to deal with them.
> 
> In another thread ("[ANN v2] Complex Camera Workshop - Tokyo - Jun,
> 19"), Mauro has mentioned a number of those:
> 
> "open source ones (Camorama, Cheese, Xawtv, Firefox, Chromium, ...) and closed
> source ones (Skype, Chrome, ...)"

Thanks for thread pointer... I may be able to get in using hangouts.

Anyway, there's big difference between open("/dev/video0") and
v4l2_open("/dev/video0"). I don't care about the first one, but yes we
should be able to support the second one eventually.

And I don't think Mauro says apps like Camorama are of open() kind.

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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> > +   while (lo < hi) {
> > +   int i = (lo + hi) / 2;
> > +   if (map->map[i].control == control) {
> > +   return map->map[i].fd;
> > +   }
> > +   if (map->map[i].control > control) {
> > +   hi = i;
> > +   continue;
> > +   }
> > +   if (map->map[i].control < control) {
> > +   lo = i+1;
> > +   continue;
> > +   }
> > +   printf("Bad: impossible condition in binary search\n");
> > +   exit(1);
> > +   }
> 
> Could we use bsearch() here?

We could, but it will mean passing function pointers etc. It would
make sense if we want to do sorting.

> > @@ -1076,18 +1115,20 @@ int v4l2_ioctl(int fd, unsigned long int request, 
> > ...)
> >ioctl, causing it to get sign extended, depending upon this 
> > behavior */
> > request = (unsigned int)request;
> >
> > +   /* FIXME */
> > if (devices[index].convert == NULL)
> > goto no_capture_request;
> >
> > /* Is this a capture request and do we need to take the stream 
> > lock? */
> > switch (request) {
> > -   case VIDIOC_QUERYCAP:
> > case VIDIOC_QUERYCTRL:
> > case VIDIOC_G_CTRL:
> > case VIDIOC_S_CTRL:
> > case VIDIOC_G_EXT_CTRLS:
> > -   case VIDIOC_TRY_EXT_CTRLS:
> > case VIDIOC_S_EXT_CTRLS:
> > +   is_subdev_request = 1;
> > +   case VIDIOC_QUERYCAP:
> > +   case VIDIOC_TRY_EXT_CTRLS:
> 
> I'm not sure why we are moving those around. Is this perhaps related
> to the FIXME comment above? If so, it would be helpful to have some
> short explanation next to it.

I want to do controls propagation only on ioctls that manipulate
controls, so I need to group them together. The FIXME comment is not
related.

> >
> > if (!is_capture_request) {
> > + int sub_fd;
> >  no_capture_request:
> > + sub_fd = fd;
> > +   if (is_subdev_request) {
> > + sub_fd = v4l2_get_fd_for_control(index, ((struct 
> > v4l2_queryctrl *) arg)->id);
> > +   }
> 
> nit: Looks like something weird going on here with indentation.

Fixed.

> > @@ -1782,3 +1828,195 @@ int v4l2_get_control(int fd, int cid)
> > (qctrl.maximum - qctrl.minimum) / 2) /
> > (qctrl.maximum - qctrl.minimum);
> >  }
> > +
> > +
> 
> nit: Double blank line.

Fixed.

> > +   if (i>=1 && map->map[i].control <= map->map[i-1].control) {
> > +   V4L2_LOG_ERR("v4l2_open_pipeline: Controls not 
> > sorted.\n");
> > +   return -1;
> 
> I guess we could just sort them at startup with qsort()?

We could... but I'd prefer them sorted on-disk, as it is written very
seldom, but needs to be readed on every device open.

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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> > > Who would be calling this function?
> > >
> > > The scenario that I could think of is:
> > > - legacy app would call open(/dev/video?), which would be handled by
> > > libv4l open hook (v4l2_open()?),
> >
> > I don't think that kind of legacy apps is in use any more. I'd prefer
> > not to deal with them.
> 
> In another thread ("[ANN v2] Complex Camera Workshop - Tokyo - Jun,
> 19"), Mauro has mentioned a number of those:
> 
> "open source ones (Camorama, Cheese, Xawtv, Firefox, Chromium, ...) and closed
> source ones (Skype, Chrome, ...)"

Yep, ok. Still would prefer not to deal with them.

(Opening additional fds behind application's back is quite nasty,
those apps should really switch to v4l2_ variants).

> > > - v4l2_open() would check if given /dev/video? figures in its list of
> > > complex pipelines, for example by calling v4l2_open_complex() and
> > > seeing if it succeeds,
> >
> > I'd rather not have v4l2_open_complex() called on devices. We could
> > test if argument is regular file and then call it... But again, that's
> > next step.
> >
> > > - if it succeeds, the resulting fd would represent the complex
> > > pipeline, otherwise it would just open the requested node directly.
> 
> What's the answer to my original question of who would be calling
> v4l2_open_complex(), then?

Application ready to deal with additional fds being
opened. contrib/test/sdlcam will be the first one.

We may do some magic to do v4l2_open_complex() in v4l2_open(), but I
believe that should be separate step.

> > >  - handling metadata CAPTURE and OUTPUT buffers controlling the 3A
> > > feedback loop - this might be optional if all we need is just ability
> > > to capture some frames, but required for getting good quality,
> > >  - actually mapping legacy controls into the above metadata,
> >
> > I'm not sure what 3A is. If you mean hardware histograms and friends,
> > yes, it would be nice to support that, but, again, statistics can be
> > computed in software.
> 
> Auto-exposure, auto-white-balance, auto-focus. In complex camera
> subsystems these need to be done in software. On most hardware
> platforms, ISP provides necessary input data (statistics) and software
> calculates required processing parameters.

Ok, so... statistics support would be nice, but that is really
separate problem.

v4l2 already contains auto-exposure and auto-white-balance. I have
patches for auto-focus. But hardware statistics are not used.

> > Yes, we'll need something more advanced.
> >
> > But.. we also need something to run the devices today, so that kernel
> > drivers can be tested and do not bitrot. That's why I'm doing this
> > work.
> 
> I guess the most important bit I missed then is what is the intended
> use case for this. It seems to be related to my earlier, unanswered
> question about who would be calliing v4l2_open_complex(), though.
> 
> What userspace applications would be used for this testing?

Main use case is kernel testing.

Secondary use case is taking .jpg photos using sdlcam.

Test apps such as qv4l2 would be nice to have, and maybe I'll
experiment with capturing video somehow one day. I'm pretty sure it
will not be easy.

Oh and I guess a link to how well it works? See
https://www.youtube.com/watch?v=fH6zuK2OOVU .

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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> Thanks for coming up with this proposal. Please see my comments below.
> 
> > Ok, can I get any comments on this one?
> > v4l2_open_complex("/file/with/descriptor", 0) can be used to open
> > whole pipeline at once, and work if it as if it was one device.
> 
> I'm not convinced if we should really be piggy backing on libv4l, but
> it's just a matter of where we put the code added by your patch, so
> let's put that aside.

There was some talk about this before, and libv4l2 is what we came
with. Only libv4l2 is in position to propagate controls to right
devices.

> Who would be calling this function?
> 
> The scenario that I could think of is:
> - legacy app would call open(/dev/video?), which would be handled by
> libv4l open hook (v4l2_open()?),

I don't think that kind of legacy apps is in use any more. I'd prefer
not to deal with them.

> - v4l2_open() would check if given /dev/video? figures in its list of
> complex pipelines, for example by calling v4l2_open_complex() and
> seeing if it succeeds,

I'd rather not have v4l2_open_complex() called on devices. We could
test if argument is regular file and then call it... But again, that's
next step.

> - if it succeeds, the resulting fd would represent the complex
> pipeline, otherwise it would just open the requested node directly.

> I guess that could give some basic camera functionality on OMAP3-like 
> hardware.

It definitely gives camera functionality on OMAP3. I'm using it to
take photos with Nokia N900.

> For most of the current generation of imaging subsystems (e.g. Intel
> IPU3, Rockchip RKISP1) it's not enough. The reason is that there is
> more to be handled by userspace than just setting controls:
>  - configuring pixel formats, resolutions, crops, etc. through the
> whole pipeline - I guess that could be preconfigured per use case
> inside the configuration file, though,

That may be future plan. Note that these can be preconfigured; unlike
controls propagation...

>  - forwarding buffers between capture and processing pipelines, i.e.
> DQBUF raw frame from CSI2 video node and QBUF to ISP video node,

My hardware does not need that, so I could not test it. I'll rely on
someone with required hardware to provide that.

(And you can take DQBUF and process it with software, at cost of
slightly higher CPU usage, right?)

>  - handling metadata CAPTURE and OUTPUT buffers controlling the 3A
> feedback loop - this might be optional if all we need is just ability
> to capture some frames, but required for getting good quality,
>  - actually mapping legacy controls into the above metadata,

I'm not sure what 3A is. If you mean hardware histograms and friends,
yes, it would be nice to support that, but, again, statistics can be
computed in software.

> I guess it's just a matter of adding further code to handle those,
> though. However, it would build up a separate legacy framework that
> locks us up into the legacy USB camera model, while we should rather
> be leaning towards a more flexible framework, such as Android Camera
> HALv3 or Pipewire. On top of such framework, we could just have a very
> thin layer to emulate the legacy, single video node, camera.

Yes, we'll need something more advanced.

But.. we also need something to run the devices today, so that kernel
drivers can be tested and do not bitrot. That's why I'm doing this
work.

And I believe we should work in steps before getting there... controls
propagation can not be done from external application, so I'm starting
with it.

> Some minor comments for the code follow.

Ok, let me send this, then go through the comments.

Best regards,
Pavel


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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

Ok, can I get any comments on this one?
v4l2_open_complex("/file/with/descriptor", 0) can be used to open
whole pipeline at once, and work if it as if it was one device.

Thanks,
Pavel

diff --git a/lib/include/libv4l2.h b/lib/include/libv4l2.h
index ea1870d..f170c3d 100644
--- a/lib/include/libv4l2.h
+++ b/lib/include/libv4l2.h
@@ -109,6 +109,12 @@ LIBV4L_PUBLIC int v4l2_get_control(int fd, int cid);
(note the fd is left open in this case). */
 LIBV4L_PUBLIC int v4l2_fd_open(int fd, int v4l2_flags);
 
+/* v4l2_open_complex: open complex pipeline. Name of pipeline descriptor
+   is passed to v4l2_open_complex(). It opens devices described there and
+   handles mapping controls between devices. 
+ */  
+LIBV4L_PUBLIC int v4l2_open_complex(char *name, int v4l2_flags);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h
index 1924c91..1ee697a 100644
--- a/lib/libv4l2/libv4l2-priv.h
+++ b/lib/libv4l2/libv4l2-priv.h
@@ -104,6 +104,7 @@ struct v4l2_dev_info {
void *plugin_library;
void *dev_ops_priv;
const struct libv4l_dev_ops *dev_ops;
+   struct v4l2_controls_map *map;
 };
 
 /* From v4l2-plugin.c */
@@ -130,4 +131,20 @@ static inline void v4l2_plugin_cleanup(void *plugin_lib, 
void *plugin_priv,
 extern const char *v4l2_ioctls[];
 void v4l2_log_ioctl(unsigned long int request, void *arg, int result);
 
+
+struct v4l2_control_map {
+   unsigned long control;
+   int fd;
+};
+
+struct v4l2_controls_map {
+   int main_fd;
+   int num_fds;
+   int num_controls;
+   struct v4l2_control_map map[];
+};
+
+int v4l2_open_pipeline(struct v4l2_controls_map *map, int v4l2_flags);
+LIBV4L_PUBLIC int v4l2_get_fd_for_control(int fd, unsigned long control);
+
 #endif
diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
index 2db25d1..39f69db 100644
--- a/lib/libv4l2/libv4l2.c
+++ b/lib/libv4l2/libv4l2.c
@@ -70,6 +70,8 @@
 #include 
 #include 
 #include 
+#include 
+
 #include "libv4l2.h"
 #include "libv4l2-priv.h"
 #include "libv4l-plugin.h"
@@ -787,6 +789,8 @@ no_capture:
if (index >= devices_used)
devices_used = index + 1;
 
+   devices[index].map = NULL;
+
/* Note we always tell v4lconvert to optimize src fmt selection for
   our default fps, the only exception is the app explicitly selecting
   a frame rate using the S_PARM ioctl after a S_FMT */
@@ -1056,12 +1060,47 @@ static int v4l2_s_fmt(int index, struct v4l2_format 
*dest_fmt)
return 0;
 }
 
+int v4l2_get_fd_for_control(int fd, unsigned long control)
+{
+   int index = v4l2_get_index(fd);
+   struct v4l2_controls_map *map;
+   int lo = 0;
+   int hi;
+
+   if (index < 0)
+   return fd;
+
+   map = devices[index].map;
+   if (!map)
+   return fd;
+   hi = map->num_controls;
+
+   while (lo < hi) {
+   int i = (lo + hi) / 2;
+   if (map->map[i].control == control) {
+   return map->map[i].fd;
+   }
+   if (map->map[i].control > control) {
+   hi = i;
+   continue;
+   }
+   if (map->map[i].control < control) {
+   lo = i+1;
+   continue;
+   }
+   printf("Bad: impossible condition in binary search\n");
+   exit(1);
+   }
+   return fd;
+}
+
 int v4l2_ioctl(int fd, unsigned long int request, ...)
 {
void *arg;
va_list ap;
int result, index, saved_err;
-   int is_capture_request = 0, stream_needs_locking = 0;
+   int is_capture_request = 0, stream_needs_locking = 0, 
+   is_subdev_request = 0;
 
va_start(ap, request);
arg = va_arg(ap, void *);
@@ -1076,18 +1115,20 @@ int v4l2_ioctl(int fd, unsigned long int request, ...)
   ioctl, causing it to get sign extended, depending upon this behavior 
*/
request = (unsigned int)request;
 
+   /* FIXME */
if (devices[index].convert == NULL)
goto no_capture_request;
 
/* Is this a capture request and do we need to take the stream lock? */
switch (request) {
-   case VIDIOC_QUERYCAP:
case VIDIOC_QUERYCTRL:
case VIDIOC_G_CTRL:
case VIDIOC_S_CTRL:
case VIDIOC_G_EXT_CTRLS:
-   case VIDIOC_TRY_EXT_CTRLS:
case VIDIOC_S_EXT_CTRLS:
+   is_subdev_request = 1;
+   case VIDIOC_QUERYCAP:
+   case VIDIOC_TRY_EXT_CTRLS:
case VIDIOC_ENUM_FRAMESIZES:
case VIDIOC_ENUM_FRAMEINTERVALS:
is_capture_request = 1;
@@ -1151,10 +1192,15 @@ int v4l2_ioctl(int fd, unsigned long int request, ...)
}
 
if (!is_capture_request) {
+ int sub_fd;
 no_capture_request:
+ sub_fd = fd;
+

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> > #modes: 2
> > Driver name: OMAP 3 resizer
> 
> It probably makes sense to place the driver name before #modes.

I dropped the driver for now. It served no purpose...

I got the patches to work on N900. With them (and right)

> From what I understood, the "#foo: " is a tag that makes the
> parser to expect for  of "foo".
> 
> I would also add a first line at the beginning describing the
> version of the format, just in case we add more stuff that
> would require to change something at the format.

Ok, done.

> Except for that, it seems ok.

Good, thanks!

Here's current version of the patch. Code will need moving to
libv4l2. Would "v4l2_open_pipeline()" be suitable name to use?

Best regards,
Pavel

diff --git a/contrib/test/sdlcam.c b/contrib/test/sdlcam.c
index cc43a10..95f85f9 100644
--- a/contrib/test/sdlcam.c
+++ b/contrib/test/sdlcam.c
@@ -8,7 +8,7 @@
Copyright 2017 Pavel Machek, LGPL
 
Needs sdl2, sdl2_image libraries. sudo aptitude install libsdl2-dev
-   libsdl2-image-dev on Debian systems.
+   libsdl2-image-dev libjpeg-dev on Debian systems.
 */
 
 #include 
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -1172,11 +1173,18 @@ static void sdl_iteration(struct sdl *m)
}
 }
 
+static int open_complex(char *name, int v4l2_flags);
+
 static void cam_open(struct dev_info *dev, char *name)
 {
struct v4l2_format *fmt = >fmt;
 
-   dev->fd = v4l2_open(name, O_RDWR);
+   //  dev->fd = v4l2_open(name, O_RDWR);
+   //dev->fd = open_complex("/my/tui/camera/n900.cv", 0);
+   dev->fd = open_complex("/data/pavel/g/v4l-utils/test.txt", 0);
+   //dev->fd = open(name, O_RDWR);
+   //printf("fd = %d\n", dev->fd);
+   //dev->fd = v4l2_fd_open(dev->fd, 0);
if (dev->fd < 0) {
printf("video %s open failed: %m\n", name);
exit(1);
@@ -1198,6 +1206,179 @@ static void cam_open(struct dev_info *dev, char *name)
 
 /* -- */
 
+static void scan_devices(char **device_names, int *device_fds, int num)
+{
+   struct dirent **namelist;
+   int n;
+   char *class_v4l = "/sys/class/video4linux";
+
+   n = scandir(class_v4l, , NULL, alphasort);
+   if (n < 0) {
+   perror("scandir");
+   return;
+   }
+   
+   while (n--) {
+   if (namelist[n]->d_name[0] != '.') {
+   char filename[1024], content[1024];
+   sprintf(filename, "%s/%s/name", class_v4l, 
namelist[n]->d_name);
+   FILE *f = fopen(filename, "r");
+   if (!f) {
+   printf("Strange, can't open %s", filename);
+   } else {
+   fgets(content, 1024, f);
+   fclose(f);
+   printf("device %s : %s\n", namelist[n]->d_name, 
content);
+   int i;
+   for (i = num-1; i >=0; i--) {
+   if (!strcmp(content, device_names[i])) {
+   sprintf(filename, "/dev/%s", 
namelist[n]->d_name);
+   device_fds[i] = open(filename, 
O_RDWR);
+   if (device_fds[i] < 0) {
+   printf("Error opening 
%s: %m\n", filename);
+   }
+   printf("*** found match - %s 
%d\n", filename, device_fds[i]);
+   }
+   }
+   }
+   }
+   free(namelist[n]);
+   }
+   free(namelist);
+  
+}
+
+static int open_complex(char *name, int v4l2_flags)
+{
+#define perr(s) printf("v4l2: open_complex: " s "\n");
+#define BUF 256
+   FILE *f = fopen(name, "r");
+
+   int res = -1;
+   char buf[BUF];
+   int version, num_modes, num_devices, num_controls;
+   int dev, control;
+
+   if (!f) {
+   perr("open of .cv file failed: %m");
+   goto err;
+   }
+
+   if (fscanf(f, "Complex Video: %d\n", ) != 1) {
+   perr(".cv file does not have required header");
+   goto close;
+   }
+
+   if (version != 0) {
+   perr(".cv file has unknown version");
+   goto close;
+   }
+  
+   if (fscanf(f, "

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> So, IMHO, entities should be described as:
> 
>   [entity entity1]
>   name = foo
>   function = bar

I don't really think windows-style config file is suitable here, as we
have more than two "nested blocks".

What about something like this? Note that I'd only implement the
controls mapping for now... but it should be extensible later to setup
mappings for the application.

Best regards,
Pavel


#modes: 2
Driver name: OMAP 3 resizer
Mode: 3000x1800
 #devices: 2
  0: et8ek8 sensor
  1: OMAP3 resizer
 #controls: 2
  0x4321a034: 1
  0x4113aab0: 1
 #links: 1
  link:
   entity1: et8ek8 sensor:1
   entity2: OMAP3 resizer:0
   resolution1: 1024x768
   resolution2: 1024x768
Mode: 1024x768
 #devices: 2
  0: et8ek8 sensor
  1: OMAP3 resizer
 #controls: 2
  0x4321a034: 1
  0x4113aab0: 1
 #links: 1
  link:
   entity1: et8ek8 sensor:1
   entity2: OMAP3 resizer:0
   resolution1: 1024x768
   resolution2: 1024x768



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


signature.asc
Description: Digital signature

[PATCH] media: i2c: lm3560: add support for lm3559 chip

[Pavel Machek]

Add support for LM3559, as found in Motorola Droid 4 phone, for
example. SW interface seems to be identical.

Signed-off-by: Pavel Machek <pa...@ucw.cz>

diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
index b600e03a..c4e5ed5 100644
--- a/drivers/media/i2c/lm3560.c
+++ b/drivers/media/i2c/lm3560.c
@@ -1,6 +1,6 @@
 /*
  * drivers/media/i2c/lm3560.c
- * General device driver for TI lm3560, FLASH LED Driver
+ * General device driver for TI lm3559, lm3560, FLASH LED Driver
  *
  * Copyright (C) 2013 Texas Instruments
  *
@@ -465,6 +479,7 @@ static int lm3560_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id lm3560_id_table[] = {
+   {LM3559_NAME, 0},
{LM3560_NAME, 0},
{}
 };
diff --git a/include/media/i2c/lm3560.h b/include/media/i2c/lm3560.h
index a5bd310..0e2b1c7 100644
--- a/include/media/i2c/lm3560.h
+++ b/include/media/i2c/lm3560.h
@@ -22,6 +22,7 @@
 
 #include 
 
+#define LM3559_NAME"lm3559"
 #define LM3560_NAME"lm3560"
 #define LM3560_I2C_ADDR(0x53)
 

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


signature.asc
Description: Digital signature

[PATCH] media: i2c: lm3560: use conservative defaults

[Pavel Machek]

If no pdata is found, we should use lowest current settings, not highest.

Signed-off-by: Pavel Machek <pa...@ucw.cz>

diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
index b600e03a..c4e5ed5 100644
--- a/drivers/media/i2c/lm3560.c
+++ b/drivers/media/i2c/lm3560.c
@@ -420,14 +434,14 @@ static int lm3560_probe(struct i2c_client *client,
pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL);
if (pdata == NULL)
return -ENODEV;
-   pdata->peak = LM3560_PEAK_3600mA;
-   pdata->max_flash_timeout = LM3560_FLASH_TOUT_MAX;
+   pdata->peak = LM3560_PEAK_1600mA;
+   pdata->max_flash_timeout = LM3560_FLASH_TOUT_MIN;
/* led 1 */
-   pdata->max_flash_brt[LM3560_LED0] = LM3560_FLASH_BRT_MAX;
-   pdata->max_torch_brt[LM3560_LED0] = LM3560_TORCH_BRT_MAX;
+   pdata->max_flash_brt[LM3560_LED0] = LM3560_FLASH_BRT_MIN;
+   pdata->max_torch_brt[LM3560_LED0] = LM3560_TORCH_BRT_MIN;
/* led 2 */
-   pdata->max_flash_brt[LM3560_LED1] = LM3560_FLASH_BRT_MAX;
-   pdata->max_torch_brt[LM3560_LED1] = LM3560_TORCH_BRT_MAX;
+   pdata->max_flash_brt[LM3560_LED1] = LM3560_FLASH_BRT_MIN;
+   pdata->max_torch_brt[LM3560_LED1] = LM3560_TORCH_BRT_MIN;
}
flash->pdata = pdata;
flash->dev = >dev;

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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> > > 2) support for running libv4l2 on mc-based devices. I'd like to do
> > > that.
> > > 
> > > Description file would look like. (# comments would not be not part of 
> > > file).
> > > 
> > > V4L2MEDIADESC
> > > 3 # number of files to open
> > > /dev/video2
> > > /dev/video6
> > > /dev/video3  
> 
> "Easy" file formats usually means maintenance troubles as new
> things are needed, and makes worse for users to write it. 
> You should take a look at lib/libdvbv5/dvb-file.c, mainly at 
> fill_entry() and dvb_read_file().

Well, file formats just need to be maintained.

> > Instead these should be entity names from the media controller.
> 
> Agreed that it should use MC. Yet, IMHO, the best would be to use
> the entity function instead, as entity names might eventually
> change on newer versions of the Kernel.

Kernel interfaces are not supposed to be changing.

> (again, user may specify just name, just function or both)
> 
> > > 3 # number of controls to map. Controls not mentioned here go to
> > >   # device 0 automatically. Sorted by control id.
> > >   # Device 0 
> > > 00980913 1
> > > 009a0003 1
> > > 009a000a 2  
> 
> I would, instead, encode it as:
> 
>   [control white balance]
>   control_id = 0x00980913
>   entity = foo_entity_name

Ok, that's really overly complex. If futrue extensibility is concern
we can do

0x00980913=whatever.

> Allowing both hexadecimal values and control macro names (can easily parsed 
> from the header file, as we already do for other things with "make
> sync").

"Easily" as in "more complex then rest of proposed code combined" :-(.

> It should probably be easy to add a generic pipeline descriptor
> with a format like:
> 
>   [pipeline pipe1]
>   link0 = SGRBG10 640x480: entity1:0 -> entity2:0[1]
>   link1 = SGRBG10 640x480: entity2:2-> entity3:0[1]
>   link2 = UYVY 640x480: entity3:1-> entity4:0[1]
>   link3 = UYVY 640x480: entity4:1-> entity5:0[1]
> 
>   sink0 = UYVY 320x200: entity5:0[1]
>   sink1 = UYVY 640x480: entity3:0[1]

Well, first, this looks like very unsuitable for key=value file. Plus,
it will not be easy to parse.  and control map needs to be
per-pipeline-configuration. Again, proposed Windows-style format can
not easily do that.

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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> >>> V4L2MEDIADESC
> >>> 3 # number of files to open
> >>> /dev/video2
> >>> /dev/video6
> >>> /dev/video3
> >>
> >> This won't work. The video nodes numbers (or even names) can change.
> >> Instead these should be entity names from the media controller.
> > 
> > Yes, it will work. 1) will configure the pipeline, and prepare
> > V4L2MEDIADESC file. The device names/numbers are stable after the
> > system is booted.
> 
> No, they are not. Drivers can be unloaded and reloaded, thus changing
> the device nodes. The media device will give you the right major/minor
> numbers and with libudev you can find the corresponding device node.
> That's the right approach.

Well, yes, drivers can be unloaded and reloaded. But at that point
pipeline will need to be set up again, so we'll get new V4L2MEDIADESC
file.

> > Yes, but that would slow down v4l2_open() needlessly. I'd prefer to
> > avoid that.
> 
> It should be quite fast. BTW, v4l2-ctl has code to find the media device
> node for a given video node.

Ok, I'll take a look, but I'd still prefer fast & simple.

> >> For that matter: what is it exactly that we want to support? I.e. where do
> >> we draw the line?
> > 
> > I'd start with fixed format first. Python prepares pipeline, and
> > provides V4L2MEDIADESC file libv4l2 can use. You can have that this
> > week.
> 
> I'm not sure I want python. An embedded system might not have python
> installed. Also, if we need to parse the configuration file in libv4l
> (and I am 90% certain that that is what we need to do), then you don't
> want to call a python script from there.

No, I don't propose calling python from libv4l2.

I propose "separate component configures the pipeline, and writes
V4L2MEDIADESC file libv4l2 then uses to map controls to devices". For
me, that component is currently python. In embededded world, I guess
they could hard-code the config, or write it in whatever language they
prefer.

> > Media control is more than 5 years old now. libv4l2 is still
> > completely uses on media control-based devices, and people are asking
> > for controls propagation in the kernel to fix that. My proposol
> > implements simple controls propagation in the userland. I believe we
> > should do that.
> 
> Your proposal implements one specific use-case. One piece of a
> larger puzzle.

Well, rather important piece, because it allows the complex cameras to
be used at all.

> >> A good test platform for this (outside the N900) is the i.MX6 platform.
> > 
> > Do you have one?
> 
> Yes. Although I would need to set it up, I still haven't done that.

Ok.

> But Steve Longerbeam is probably the right person to test this for you.
> 
> What I have been thinking of (although never in any real detail) is that we
> probably want to have an application that is run from udev when a media device
> is created and that reads a config file and does an initial pipeline 
> configuration.
> 
> So once that's setup you should be able to do: 'v4l2-ctl --stream-mmap' and
> get video.
> 
> Next, libv4l will also read the same configuration file and use it to provide
> a compatibility layer so applications that use libv4l will work better with
> MC devices. Part of that is indeed control handling.

Yep, that would be nice. And yes, control handling is important for
me.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> >> I really want to work with you on this, but I am not looking for partial
> >> solutions.
> > 
> > Well, expecting design to be done for opensource development is a bit
> > unusual :-).
> 
> Why? We have done that quite often in the past. Media is complex and you need
> to decide on a design up front.



> > I really see two separate tasks
> > 
> > 1) support for configuring pipeline. I believe this is best done out
> > of libv4l2. It outputs description file, format below. Currently I
> > have implemented this is in Python. File format is below.
> 
> You do need this, but why outside of libv4l2? I'm not saying I disagree
> with you, but you need to give reasons for that.

I'd prefer to do this in Python. There's a lot to configure there, and
I'm not sure if libv4l2 is is right place for it. Anyway, design of 2)
does not depend on this.

> > 2) support for running libv4l2 on mc-based devices. I'd like to do
> > that.
> > 
> > Description file would look like. (# comments would not be not part of 
> > file).
> > 
> > V4L2MEDIADESC
> > 3 # number of files to open
> > /dev/video2
> > /dev/video6
> > /dev/video3
> 
> This won't work. The video nodes numbers (or even names) can change.
> Instead these should be entity names from the media controller.

Yes, it will work. 1) will configure the pipeline, and prepare
V4L2MEDIADESC file. The device names/numbers are stable after the
system is booted.

If these were entity names, v4l2_open() would have to go to /sys and
search for corresponding files... which would be rather complex and
slow.

> > 3 # number of controls to map. Controls not mentioned here go to
> >   # device 0 automatically. Sorted by control id.
> >   # Device 0 
> > 00980913 1
> > 009a0003 1
> > 009a000a 2
> 
> You really don't need to specify the files to open. All you need is to
> specify the entity ID and the list of controls that you need.
> 
> Then libv4l can just figure out which device node(s) to open for
> that.

Yes, but that would slow down v4l2_open() needlessly. I'd prefer to
avoid that.

> > We can parse that easily without requiring external libraries. Sorted
> > data allow us to do binary search.
> 
> But none of this addresses setting up the initial video pipeline or
> changing formats. We probably want to support that as well.

Well, maybe one day. But I don't believe we should attempt to support
that today.

Currently, there's no way to test that camera works on N900 with
mainline v4l2... which is rather sad. Advanced use cases can come later.

> For that matter: what is it exactly that we want to support? I.e. where do
> we draw the line?

I'd start with fixed format first. Python prepares pipeline, and
provides V4L2MEDIADESC file libv4l2 can use. You can have that this
week.

I guess it would make sense to support "application says preffered
resolution, libv4l2 attempts to set up some kind of pipeline to get
that resolution", but yes, interface there will likely be quite
complex.

Media control is more than 5 years old now. libv4l2 is still
completely uses on media control-based devices, and people are asking
for controls propagation in the kernel to fix that. My proposol
implements simple controls propagation in the userland. I believe we
should do that.

> A good test platform for this (outside the N900) is the i.MX6 platform.

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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> > Pavel,
> > 
> > I appreciate your efforts of adding support for mc-based devices to
> > libv4l.

Thanks.

> > I guess the main poin that Hans is pointing is that we should take
> > extra care in order to avoid adding new symbols to libv4l ABI/API
> > without being sure that they'll be needed in long term, as removing
> > or changing the API is painful for app developers, and keeping it
> > ABI compatible with apps compiled against previous versions of the
> > library is very painful for us.
> 
> Indeed. Sorry if I wasn't clear on that.

Aha, ok, no, I did not get that.

> > The hole idea is that generic applications shouldn't notice
> > if the device is using a mc-based device or not.
> 
> What is needed IMHO is an RFC that explains how you want to solve this
> problem, what the parser would look like, how this would configure a
> complex pipeline for use with libv4l-using applications, etc.
> 
> I.e., a full design.
> 
> And once everyone agrees that that design is solid, then it needs to be
> implemented.
> 
> I really want to work with you on this, but I am not looking for partial
> solutions.

Well, expecting design to be done for opensource development is a bit
unusual :-).

I really see two separate tasks

1) support for configuring pipeline. I believe this is best done out
of libv4l2. It outputs description file, format below. Currently I
have implemented this is in Python. File format is below.

2) support for running libv4l2 on mc-based devices. I'd like to do
that.

Description file would look like. (# comments would not be not part of file).

V4L2MEDIADESC
3 # number of files to open
/dev/video2
/dev/video6
/dev/video3
3 # number of controls to map. Controls not mentioned here go to
  # device 0 automatically. Sorted by control id.
  # Device 0 
00980913 1
009a0003 1
009a000a 2

We can parse that easily without requiring external libraries. Sorted
data allow us to do binary search.

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


signature.asc
Description: Digital signature

Re: [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

> I really don't want to add functions for this to libv4l2. That's just a
> quick hack. The real solution is to parse this from a config
> file. But

No, this is not a quick hack. These are functions that will eventually
be used by the config parser. (Oh and they allow me to use camera on
complex hardware, but...).

Hmm, I have mentioned that already. See quoted text below. 

> that is a lot more work and it is something that needs to be designed
> properly.
> 
> And that requires someone to put in the time and effort...

Which is what I'm trying to do. But some cooperation from your side is
needed, too. I acknowledged some kind of parser is needed. I can
do that. Are you willing to cooperate?

But I need your feedback on the parts below. We can bikeshed about the
parser later.

Do they look acceptable? Did I hook up right functions in acceptable
way?

If so, yes, I can proceed with parser.

Best regards,
Pavel


> > ...so you can use it on complex devices. Tested on my N900.
> > 
> > I guess later helper would be added that would parse some kind of
> > descritption file and do open_pipeline(). But.. lets solve that
> > next. In the first place, it would be nice to have libv4l2 usable on
> > complex devices.
> > 
> > Best regards,
> >         Pavel
> > Signed-off-by: Pavel Machek <pa...@ucw.cz>
> > 
> > diff --git a/lib/include/libv4l2.h b/lib/include/libv4l2.h
> > index ea1870d..6220dfd 100644
> > --- a/lib/include/libv4l2.h
> > +++ b/lib/include/libv4l2.h
> > @@ -109,6 +109,23 @@ LIBV4L_PUBLIC int v4l2_get_control(int fd, int cid);
> > (note the fd is left open in this case). */
> >  LIBV4L_PUBLIC int v4l2_fd_open(int fd, int v4l2_flags);
> >  
> > +struct v4l2_control_map {
> > +   unsigned long control;
> > +   int fd;
> > +};
> > +
> > +struct v4l2_controls_map {
> > +   int main_fd;
> > +   int num_fds;
> > +   int num_controls;
> > +   struct v4l2_control_map map[];
> > +};
> > +
> > +LIBV4L_PUBLIC int v4l2_open_pipeline(struct v4l2_controls_map *map, int 
> > v4l2_flags);
> > +
> > +LIBV4L_PUBLIC int v4l2_get_fd_for_control(int fd, unsigned long control);
> > +
> > +
> >  #ifdef __cplusplus
> >  }
> >  #endif /* __cplusplus */
> > diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h
> > index 1924c91..ebe5dad 100644
> > --- a/lib/libv4l2/libv4l2-priv.h
> > +++ b/lib/libv4l2/libv4l2-priv.h
> > @@ -104,6 +104,7 @@ struct v4l2_dev_info {
> > void *plugin_library;
> > void *dev_ops_priv;
> > const struct libv4l_dev_ops *dev_ops;
> > +   struct v4l2_controls_map *map;
> >  };
> >  
> >  /* From v4l2-plugin.c */
> > diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
> > index 2db25d1..b3ae70b 100644
> > --- a/lib/libv4l2/libv4l2.c
> > +++ b/lib/libv4l2/libv4l2.c
> > @@ -787,6 +787,8 @@ no_capture:
> > if (index >= devices_used)
> > devices_used = index + 1;
> >  
> > +   devices[index].map = NULL;
> > +
> > /* Note we always tell v4lconvert to optimize src fmt selection for
> >our default fps, the only exception is the app explicitly selecting
> >a frame rate using the S_PARM ioctl after a S_FMT */
> > @@ -1056,12 +1058,39 @@ static int v4l2_s_fmt(int index, struct v4l2_format 
> > *dest_fmt)
> > return 0;
> >  }
> >  
> > +int v4l2_get_fd_for_control(int fd, unsigned long control)
> > +{
> > +   int index = v4l2_get_index(fd);
> > +   struct v4l2_controls_map *map = devices[index].map;
> > +   int lo = 0;
> > +   int hi = map->num_controls;
> > +
> > +   while (lo < hi) {
> > +   int i = (lo + hi) / 2;
> > +   if (map->map[i].control == control) {
> > +   return map->map[i].fd + fd;
> > +   }
> > +   if (map->map[i].control > control) {
> > +   hi = i;
> > +   continue;
> > +   }
> > +   if (map->map[i].control < control) {
> > +   lo = i+1;
> > +   continue;
> > +   }
> > +   printf("Bad: impossible condition in binary search\n");
> > +   exit(1);
> > +   }
> > +   return fd;
> > +}
> > +
> >  int v4l2_ioctl(int fd, unsigned long int request, ...)
> >  {
> > void *arg;
> > va_list ap;
> > 

[RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline

[Pavel Machek]

Hi!

What about something like this?

Pass map of controls and expects fds to libv4l2...
 
+static struct v4l2_controls_map map = {
+  .num_fds = 2,
+  .num_controls = 3,
+  .map = { [0] = { .control = 0x00980913, .fd = 1 },
+  [1] = { .control = V4L2_CID_EXPOSURE_ABSOLUTE, .fd = 1 },
+  [2] = { .control = V4L2_CID_FOCUS_ABSOLUTE, .fd = 2 },
+  
+ },
+};
+
+static void open_chk(char *name, int fd)
+{
+   int f = open(name, O_RDWR);
+
+   if (fd != f) {
+ printf("Unexpected error %m opening %s\n", name);
+ exit(1);
+   }
+}
+
+static void cam_open_n900(struct dev_info *dev)
+{
+   struct v4l2_format *fmt = >fmt;
+   int fd;
+   
+   map.main_fd = open("/dev/video_ccdc", O_RDWR);
+
+   open_chk("/dev/video_sensor", map.main_fd+1);
+   open_chk("/dev/video_focus", map.main_fd+2);
+   //  open_chk("/dev/video_flash", map.main_fd+3);
+
+   v4l2_open_pipeline(, 0);
+   dev->fd = map.main_fd;
+

...so you can use it on complex devices. Tested on my N900.

I guess later helper would be added that would parse some kind of
descritption file and do open_pipeline(). But.. lets solve that
next. In the first place, it would be nice to have libv4l2 usable on
complex devices.

Best regards,
        Pavel
Signed-off-by: Pavel Machek <pa...@ucw.cz>

diff --git a/lib/include/libv4l2.h b/lib/include/libv4l2.h
index ea1870d..6220dfd 100644
--- a/lib/include/libv4l2.h
+++ b/lib/include/libv4l2.h
@@ -109,6 +109,23 @@ LIBV4L_PUBLIC int v4l2_get_control(int fd, int cid);
(note the fd is left open in this case). */
 LIBV4L_PUBLIC int v4l2_fd_open(int fd, int v4l2_flags);
 
+struct v4l2_control_map {
+   unsigned long control;
+   int fd;
+};
+
+struct v4l2_controls_map {
+   int main_fd;
+   int num_fds;
+   int num_controls;
+   struct v4l2_control_map map[];
+};
+
+LIBV4L_PUBLIC int v4l2_open_pipeline(struct v4l2_controls_map *map, int 
v4l2_flags);
+
+LIBV4L_PUBLIC int v4l2_get_fd_for_control(int fd, unsigned long control);
+
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h
index 1924c91..ebe5dad 100644
--- a/lib/libv4l2/libv4l2-priv.h
+++ b/lib/libv4l2/libv4l2-priv.h
@@ -104,6 +104,7 @@ struct v4l2_dev_info {
void *plugin_library;
void *dev_ops_priv;
const struct libv4l_dev_ops *dev_ops;
+   struct v4l2_controls_map *map;
 };
 
 /* From v4l2-plugin.c */
diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
index 2db25d1..b3ae70b 100644
--- a/lib/libv4l2/libv4l2.c
+++ b/lib/libv4l2/libv4l2.c
@@ -787,6 +787,8 @@ no_capture:
if (index >= devices_used)
devices_used = index + 1;
 
+   devices[index].map = NULL;
+
/* Note we always tell v4lconvert to optimize src fmt selection for
   our default fps, the only exception is the app explicitly selecting
   a frame rate using the S_PARM ioctl after a S_FMT */
@@ -1056,12 +1058,39 @@ static int v4l2_s_fmt(int index, struct v4l2_format 
*dest_fmt)
return 0;
 }
 
+int v4l2_get_fd_for_control(int fd, unsigned long control)
+{
+   int index = v4l2_get_index(fd);
+   struct v4l2_controls_map *map = devices[index].map;
+   int lo = 0;
+   int hi = map->num_controls;
+
+   while (lo < hi) {
+   int i = (lo + hi) / 2;
+   if (map->map[i].control == control) {
+   return map->map[i].fd + fd;
+   }
+   if (map->map[i].control > control) {
+   hi = i;
+   continue;
+   }
+   if (map->map[i].control < control) {
+   lo = i+1;
+   continue;
+   }
+   printf("Bad: impossible condition in binary search\n");
+   exit(1);
+   }
+   return fd;
+}
+
 int v4l2_ioctl(int fd, unsigned long int request, ...)
 {
void *arg;
va_list ap;
int result, index, saved_err;
-   int is_capture_request = 0, stream_needs_locking = 0;
+   int is_capture_request = 0, stream_needs_locking = 0, 
+   is_subdev_request = 0;
 
va_start(ap, request);
arg = va_arg(ap, void *);
@@ -1076,18 +1105,20 @@ int v4l2_ioctl(int fd, unsigned long int request, ...)
   ioctl, causing it to get sign extended, depending upon this behavior 
*/
request = (unsigned int)request;
 
+   /* FIXME */
if (devices[index].convert == NULL)
goto no_capture_request;
 
/* Is this a capture request and do we need to take the stream lock? */
switch (request) {
-   case VIDIOC_QUERYCAP:
case VIDIOC_QUERYCTRL:
case VIDIOC_G_CTRL:
case VIDIOC_S_CTRL:

Re: [PATCH v2] media: omap3isp: fix unbalanced dma_iommu_mapping

[Pavel Machek]

On Wed 2018-03-14 10:41:36, Suman Anna wrote:
> The OMAP3 ISP driver manages its MMU mappings through the IOMMU-aware
> ARM DMA backend. The current code creates a dma_iommu_mapping and
> attaches this to the ISP device, but never detaches the mapping in
> either the probe failure paths or the driver remove path resulting
> in an unbalanced mapping refcount and a memory leak. Fix this properly.
> 
> Reported-by: Pavel Machek <pa...@ucw.cz>
> Signed-off-by: Suman Anna <s-a...@ti.com>
> Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> ---
> v2 Changes:
>  - Dropped the error_attach label, and returned directly from the
>first error path (comments from Sakari)
>  - Added Sakari's Acked-by
> v1: https://patchwork.kernel.org/patch/10276759/
> 
> Pavel,
> I dropped your Tested-by from v2 since I modified the patch, can you
> recheck the new patch again? Thanks.

I updated to new -next version and re-ran the test.

Tested-by: Pavel Machek <pa...@ucw.cz>
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

next-20180219: camera problems on n900

[Pavel Machek]

Hi!

> > > > In v4.14, back camera on N900 works. On v4.15-rc1.. it works for few
> > > > seconds, but then I get repeated oopses.
> > > > 
> > > > On v4.15-rc0.5 (commit ed30b147e1f6e396e70a52dbb6c7d66befedd786),
> > > > camera does not start.
> > > > 
> > > > Any ideas what might be wrong there?
> > > 
> > > What kind of oopses do you get?
> > 
> > Hmm. bisect pointed to commit that can't be responsible Ideas
> > welcome.
> 
> Hi Pavel,
> 
> I tested N9 and capture appears to be working from the CSI-2 receiver
> (media tree master, i.e. v4.15-rc3 now).
> 
> Which pipeline did you use?

I tested next-20180219 and camera does not work there. It leads to
nasty oops. I tried to capture the oops with dmesg > file, and that
one oopsed, too. usb networking also has problems... fun.

Camera works ok in V4.16-rc1, but if I use the camera, next shutdown
will oops. I'll try to collect more data there, too.

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


signature.asc
Description: Digital signature

Re: [PATCH v2 8/8] platform: vivid-cec: use 64-bit arithmetic instead of 32-bit

[Pavel Machek]

On Mon 2018-02-05 22:29:41, Hans Verkuil wrote:
> On 02/05/2018 09:36 PM, Gustavo A. R. Silva wrote:
> > Add suffix ULL to constant 10 in order to give the compiler complete
> > information about the proper arithmetic to use. Notice that this
> > constant is used in a context that expects an expression of type
> > u64 (64 bits, unsigned).
> > 
> > The expression len * 10 * CEC_TIM_DATA_BIT_TOTAL is currently being
> > evaluated using 32-bit arithmetic.
> > 
> > Also, remove unnecessary parentheses and add a code comment to make it
> > clear what is the reason of the code change.
> > 
> > Addresses-Coverity-ID: 1454996
> > Signed-off-by: Gustavo A. R. Silva 
> > ---
> > Changes in v2:
> >  - Update subject and changelog to better reflect the proposed code changes.
> >  - Add suffix ULL to constant instead of casting a variable.
> >  - Remove unncessary parentheses.
> 
> unncessary -> unnecessary
> 
> >  - Add code comment.
> > 
> >  drivers/media/platform/vivid/vivid-cec.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/platform/vivid/vivid-cec.c 
> > b/drivers/media/platform/vivid/vivid-cec.c
> > index b55d278..614787b 100644
> > --- a/drivers/media/platform/vivid/vivid-cec.c
> > +++ b/drivers/media/platform/vivid/vivid-cec.c
> > @@ -82,8 +82,15 @@ static void vivid_cec_pin_adap_events(struct cec_adapter 
> > *adap, ktime_t ts,
> >  
> > if (adap == NULL)
> > return;
> > -   ts = ktime_sub_us(ts, (CEC_TIM_START_BIT_TOTAL +
> > -  len * 10 * CEC_TIM_DATA_BIT_TOTAL));
> > +
> > +   /*
> > +* Suffix ULL on constant 10 makes the expression
> > +* CEC_TIM_START_BIT_TOTAL + 10ULL * len * CEC_TIM_DATA_BIT_TOTAL
> > +* be evaluated using 64-bit unsigned arithmetic (u64), which
> > +* is what ktime_sub_us expects as second argument.
> > +*/
> 
> That's not really the comment that I was looking for. It still doesn't
> explain *why* this is needed at all. How about something like this:
> 
> /*
>  * Add the ULL suffix to the constant 10 to work around a false Coverity
>  * "Unintentional integer overflow" warning. Coverity isn't smart enough
>  * to understand that len is always <= 16, so there is no chance of an
>  * integer overflow.
>  */

Or maybe it would be better to add comment about Coverity having
false-positive and not to modify the code?

Hmm. Could we do something like BUG_ON(len > 16) to make Coverity
understand the ranges?

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


signature.asc
Description: Digital signature

Re: v4.15: camera problems on n900

[Pavel Machek]

On Fri 2017-12-29 11:38:55, Sakari Ailus wrote:
> On Thu, Dec 28, 2017 at 09:24:53PM +0100, Pavel Machek wrote:
> > On Wed 2017-12-27 23:17:19, Sakari Ailus wrote:
> > > On Wed, Dec 27, 2017 at 10:05:43PM +0100, Pavel Machek wrote:
> > > > Hi!
> > > > 
> > > > In v4.14, back camera on N900 works. On v4.15-rc1.. it works for few
> > > > seconds, but then I get repeated oopses.
> > > > 
> > > > On v4.15-rc0.5 (commit ed30b147e1f6e396e70a52dbb6c7d66befedd786),
> > > > camera does not start.
> > > > 
> > > > Any ideas what might be wrong there?
> > > 
> > > What kind of oopses do you get?
> > 
> > Hmm. bisect pointed to commit that can't be responsible Ideas
> > welcome.
> 
> Hi Pavel,
> 
> I tested N9 and capture appears to be working from the CSI-2 receiver
> (media tree master, i.e. v4.15-rc3 now).
> 
> Which pipeline did you use?

I'm using the "main" sensor (5MPx) and am using capture (not preview)
pipeline.

I tested linux-next in the meantime, and that one seems to
work. (Which is strange, I believe it oopsed before. Perhaps something
random?) With linux-next working, it should not be hardto figure out
what is going on.

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


signature.asc
Description: Digital signature

Re: v4.15: camera problems on n900

[Pavel Machek]

On Wed 2017-12-27 23:17:19, Sakari Ailus wrote:
> On Wed, Dec 27, 2017 at 10:05:43PM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > In v4.14, back camera on N900 works. On v4.15-rc1.. it works for few
> > seconds, but then I get repeated oopses.
> > 
> > On v4.15-rc0.5 (commit ed30b147e1f6e396e70a52dbb6c7d66befedd786),
> > camera does not start.
> > 
> > Any ideas what might be wrong there?
> 
> What kind of oopses do you get?

Hmm. bisect pointed to commit that can't be responsible Ideas
welcome.


Pavel

# bad: [fb3f95c11904adf26c2bd86fe1b1613c921710b5] Config for v4.15-rc0.5
# good: [c213cf57c2f15ee226c14dd7157caa334c3ef7c8] Make config similar to n950 
case. Still works on n900.
git bisect start 'mini-v4.15' 'mini-v4.14'
# good: [06410bdec961a55e78e01d4fda199f709a84e17f] Merge /data/l/clean-cg into 
mini-v4.14
git bisect good 06410bdec961a55e78e01d4fda199f709a84e17f
# bad: [fc35c1966e1372a21a88f6655279361e2f92713f] Merge tag 'clk-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux
git bisect bad fc35c1966e1372a21a88f6655279361e2f92713f
# good: [bebc6082da0a9f5d47a1ea2edc099bf671058bd4] Linux 4.14
git bisect good bebc6082da0a9f5d47a1ea2edc099bf671058bd4
# good: [5bbcc0f595fadb4cac0eddc4401035ec0bd95b09] Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next
git bisect good 5bbcc0f595fadb4cac0eddc4401035ec0bd95b09
# bad: [5b0e2cb020085efe202123162502e0b551e49a0e] Merge tag 'powerpc-4.15-1' of 
git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
git bisect bad 5b0e2cb020085efe202123162502e0b551e49a0e
# good: [f150891fd9878ef0d9197c4e8451ce67c3bdd014] Merge tag 
'exynos-drm-next-for-v4.15' of 
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-next
git bisect good f150891fd9878ef0d9197c4e8451ce67c3bdd014
# good: [93ea0eb7d77afab34657715630d692a78b8cea6a] Merge tag 'leaks-4.15-rc1' 
of git://github.com/tcharding/linux
git bisect good 93ea0eb7d77afab34657715630d692a78b8cea6a
# bad: [2bf16b7a73caf3435f782e4170cfe563675e10f9] Merge tag 
'char-misc-4.15-rc1' of 
ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
git bisect bad 2bf16b7a73caf3435f782e4170cfe563675e10f9
# good: [ef674997e49760137ca9a90aac41a9922ac399b2] media: staging: atomisp: 
Convert timers to use timer_setup()
git bisect good ef674997e49760137ca9a90aac41a9922ac399b2
# good: [b1cb7372fa822af6c06c8045963571d13ad6348b] dvb_frontend: don't 
use-after-free the frontend struct
git bisect good b1cb7372fa822af6c06c8045963571d13ad6348b
# good: [c9fe0f8fa4136c2451dcc012e48fbf4470d6b592] hyper-v: trace 
vmbus_on_msg_dpc()
git bisect good c9fe0f8fa4136c2451dcc012e48fbf4470d6b592
# good: [e20d2b291ba2f5441fd4aacd746c21e60d48b559] nvmem: imx-ocotp: Pass 
parameters via a struct
git bisect good e20d2b291ba2f5441fd4aacd746c21e60d48b559
# good: [0ff26c662d5f3b26674d5205c8899d901f766acb] driver core: Fix device link 
deferred probe
git bisect good 0ff26c662d5f3b26674d5205c8899d901f766acb
# good: [d4035a8c1ff7288af9e47d6d05384f14c9308ea1] MAINTAINERS: Update VME 
subsystem tree.
git bisect good d4035a8c1ff7288af9e47d6d05384f14c9308ea1
# bad: [e60e1ee60630cafef5e430c2ae364877e061d980] Merge tag 'drm-for-v4.15' of 
git://people.freedesktop.org/~airlied/linux
git bisect bad e60e1ee60630cafef5e430c2ae364877e061d980
# bad: [5d352e69c60e54b5f04d6e337a1d2bf0dbf3d94a] Merge tag 'media/v4.15-1' of 
ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
git bisect bad 5d352e69c60e54b5f04d6e337a1d2bf0dbf3d94a
# good: [f2ecc3d0787e05d9145722feed01d4a11ab6bec1] Merge tag 'staging-4.15-rc1' 
into v4l_for_linus
git bisect good f2ecc3d0787e05d9145722feed01d4a11ab6bec1
# first bad commit: [5d352e69c60e54b5f04d6e337a1d2bf0dbf3d94a] Merge tag 
'media/v4.15-1' of 
ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media


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


signature.asc
Description: Digital signature

Re: v4.15: camera problems on n900

[Pavel Machek]

On Wed 2017-12-27 23:17:19, Sakari Ailus wrote:
> On Wed, Dec 27, 2017 at 10:05:43PM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > In v4.14, back camera on N900 works. On v4.15-rc1.. it works for few
> > seconds, but then I get repeated oopses.
> > 
> > On v4.15-rc0.5 (commit ed30b147e1f6e396e70a52dbb6c7d66befedd786),
> > camera does not start.
> > 
> > Any ideas what might be wrong there?
> 
> What kind of oopses do you get?

Haven't seen the oopses yet; maybe they are only on linux-next?
Anyway, bisect so far:

# bad: [fb3f95c11904adf26c2bd86fe1b1613c921710b5] Config for v4.15-rc0.5
# good: [c213cf57c2f15ee226c14dd7157caa334c3ef7c8] Make config similar to n950 
case. Still works on n900.
git bisect start 'mini-v4.15' 'mini-v4.14'
# good: [06410bdec961a55e78e01d4fda199f709a84e17f] Merge /data/l/clean-cg into 
mini-v4.14
git bisect good 06410bdec961a55e78e01d4fda199f709a84e17f
# bad: [fc35c1966e1372a21a88f6655279361e2f92713f] Merge tag 'clk-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux
git bisect bad fc35c1966e1372a21a88f6655279361e2f92713f
# good: [bebc6082da0a9f5d47a1ea2edc099bf671058bd4] Linux 4.14
git bisect good bebc6082da0a9f5d47a1ea2edc099bf671058bd4
# good: [5bbcc0f595fadb4cac0eddc4401035ec0bd95b09] Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next
git bisect good 5bbcc0f595fadb4cac0eddc4401035ec0bd95b09
# bad: [5b0e2cb020085efe202123162502e0b551e49a0e] Merge tag 'powerpc-4.15-1' of 
git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
git bisect bad 5b0e2cb020085efe202123162502e0b551e49a0e
# good: [f150891fd9878ef0d9197c4e8451ce67c3bdd014] Merge tag 
'exynos-drm-next-for-v4.15' of 
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-next
git bisect good f150891fd9878ef0d9197c4e8451ce67c3bdd014
# good: [93ea0eb7d77afab34657715630d692a78b8cea6a] Merge tag 'leaks-4.15-rc1' 
of git://github.com/tcharding/linux
git bisect good 93ea0eb7d77afab34657715630d692a78b8cea6a
# bad: [2bf16b7a73caf3435f782e4170cfe563675e10f9] Merge tag 
'char-misc-4.15-rc1' of 
ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
git bisect bad 2bf16b7a73caf3435f782e4170cfe563675e10f9
# good: [ef674997e49760137ca9a90aac41a9922ac399b2] media: staging: atomisp: 
Convert timers to use timer_setup()
git bisect good ef674997e49760137ca9a90aac41a9922ac399b2
# good: [b1cb7372fa822af6c06c8045963571d13ad6348b] dvb_frontend: don't 
use-after-free the frontend struct
git bisect good b1cb7372fa822af6c06c8045963571d13ad6348b


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


signature.asc
Description: Digital signature

Re: v4.15: camera problems on n900

[Pavel Machek]

1;2802;0cOn Wed 2017-12-27 23:17:19, Sakari Ailus wrote:
> On Wed, Dec 27, 2017 at 10:05:43PM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > In v4.14, back camera on N900 works. On v4.15-rc1.. it works for few
> > seconds, but then I get repeated oopses.
> > 
> > On v4.15-rc0.5 (commit ed30b147e1f6e396e70a52dbb6c7d66befedd786),
> > camera does not start.
> > 
> > Any ideas what might be wrong there?
> 
> What kind of oopses do you get?

They seemed to be in unrelated processes -> not useful for
debugging. I tried again, but this time it hangs, similar way to
-rc0.5. (That might be good news).

Does it work for you on N9?

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


signature.asc
Description: Digital signature

v4.15: camera problems on n900

[Pavel Machek]

Hi!

In v4.14, back camera on N900 works. On v4.15-rc1.. it works for few
seconds, but then I get repeated oopses.

On v4.15-rc0.5 (commit ed30b147e1f6e396e70a52dbb6c7d66befedd786),
camera does not start.

Any ideas what might be wrong there?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH] devicetree: Add video bus switch

[Pavel Machek]

Hi!

> > > I don't really object using g_endpoint_config() as a temporary solution;
> > > I'd like to have Laurent's opinion on that though. Another option is to
> > > wait, but we've already waited a looong time (as in total).
> > 
> > Laurent, do you have some input here? We have simple "2 cameras
> > connected to one signal processor" situation here. We need some way of
> > passing endpoint configuration from the sensors through the switch. I
> > did this:
> 
> Could you give me a bit more information about the platform you're targeting: 
> how the switch is connected, what kind of switch it is, and what endpoint 
> configuration data you need ?

Platform is Nokia N900, Ivaylo already gave pointer to schematics.

Switch is controlled using GPIO, and basically there's CSI
configuration that would normally be in the device tree, but now we
have two CSI configurations to select from...

> > 9) Highly reconfigurable hardware - Julien Beraud
> > 
> > - 44 sub-devices connected with an interconnect.
> > - As long as formats match, any sub-device could be connected to any
> > - other sub-device through a link.
> > - The result is 44 * 44 links at worst.
> > - A switch sub-device proposed as the solution to model the
> > - interconnect. The sub-devices are connected to the switch
> > - sub-devices through the hardware links that connect to the
> > - interconnect.
> > - The switch would be controlled through new IOCTLs S_ROUTING and
> > - G_ROUTING.
> > - Patches available:
> >  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> > 
> > but the patches are from 2005. So I guess I'll need some guidance here...
> 
> You made me feel very old for a moment. The patches are from 2015 :-)

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


signature.asc
Description: Digital signature

Re: [PATCH] media: et8ek8: select V4L2_FWNODE

[Pavel Machek]

On Mon 2017-11-13 14:56:45, Arnd Bergmann wrote:
> v4l2_async_register_subdev_sensor_common() is only provided when
> CONFIG_V4L2_FWNODE is enabled, otherwise we get a link failure:
> 
> drivers/media/i2c/et8ek8/et8ek8_driver.o: In function `et8ek8_probe':
> et8ek8_driver.c:(.text+0x884): undefined reference to 
> `v4l2_async_register_subdev_sensor_common'
> 
> This adds a Kconfig 'select' statement like all the other users of
> this interface have.
> 
> Fixes: d8932f38c10f ("media: et8ek8: Add support for flash and lens devices")
> Signed-off-by: Arnd Bergmann <a...@arndb.de>

Acked-by: Pavel Machek <pa...@ucw.cz>

Thanks!
Pavel

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


signature.asc
Description: Digital signature

Re: [PATCH 1/2] sdlcam: fix linking

[Pavel Machek]

On Mon 2017-11-13 10:19:07, Rafaël Carré wrote:
> Signed-off-by: Rafaël Carré <fun...@videolan.org>

Acked-by: Pavel Machek <pa...@ucw.cz>

Thanks!
Pavel


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


signature.asc
Description: Digital signature

[rfc] libv4l2: better auto-gain

[Pavel Machek]

Add support for better autogain. Old code had average brightness as a
target. New code has number of bright pixels as a target.

Signed-off-by: Pavel Machek <pa...@ucw.cz>

I see I need to implement histogram for bayer8 and rgb24. Any other
comments?

Best regards,
Pavel

diff --git a/lib/libv4lconvert/processing/autogain.c 
b/lib/libv4lconvert/processing/autogain.c
index c6866d6..a2c69f4 100644
--- a/lib/libv4lconvert/processing/autogain.c
+++ b/lib/libv4lconvert/processing/autogain.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "libv4lprocessing.h"
 #include "libv4lprocessing-priv.h"
@@ -40,179 +41,136 @@ static int autogain_active(struct v4lprocessing_data 
*data)
return autogain;
 }
 
-/* Adjust ctrl value with steps steps, while not crossing limit */
-static void autogain_adjust(struct v4l2_queryctrl *ctrl, int *value,
-   int steps, int limit, int accel)
+#define BUCKETS 20
+
+static void v4l2_histogram_bayer10(unsigned short *buf, int cdf[], const 
struct v4l2_format *fmt)
 {
-   int ctrl_range = (ctrl->maximum - ctrl->minimum) / ctrl->step;
-
-   /* If we are of 3 * deadzone or more, and we have a fine grained
-  control, take larger steps, otherwise we take ages to get to the
-  right setting point. We use 256 as tripping point for determining
-  fine grained controls here, as avg_lum has a range of 0 - 255. */
-   if (accel && abs(steps) >= 3 && ctrl_range > 256)
-   *value += steps * ctrl->step * (ctrl_range / 256);
-/* If we are of by less then 3, but have a very finegrained control
-   still speed things up a bit */
-   else if (accel && ctrl_range > 1024)
-   *value += steps * ctrl->step * (ctrl_range / 1024);
-   else
-   *value += steps * ctrl->step;
-
-   if (steps > 0) {
-   if (*value > limit)
-   *value = limit;
-   } else {
-   if (*value < limit)
-   *value = limit;
-   }
+   for (int y = 0; y < fmt->fmt.pix.height; y+=19)
+   for (int x = 0; x < fmt->fmt.pix.width; x+=19) {
+   int b;
+   b = buf[fmt->fmt.pix.width*y + x];
+   b = (b * BUCKETS)/(1024);
+   cdf[b]++;
+   }
 }
 
-/* auto gain and exposure algorithm based on the knee algorithm described here:
-http://ytse.tricolour.net/docs/LowLightOptimization.html */
-static int autogain_calculate_lookup_tables(
-   struct v4lprocessing_data *data,
-   unsigned char *buf, const struct v4l2_format *fmt)
+static int v4l2_s_ctrl(int fd, long id, long value)
 {
-   int x, y, target, steps, avg_lum = 0;
-   int gain, exposure, orig_gain, orig_exposure, exposure_low;
+int res;
struct v4l2_control ctrl;
-   struct v4l2_queryctrl gainctrl, expoctrl;
-   const int deadzone = 6;
-
-   ctrl.id = V4L2_CID_EXPOSURE;
-   expoctrl.id = V4L2_CID_EXPOSURE;
-   if (SYS_IOCTL(data->fd, VIDIOC_QUERYCTRL, ) ||
-   SYS_IOCTL(data->fd, VIDIOC_G_CTRL, ))
-   return 0;
-
-   exposure = orig_exposure = ctrl.value;
-   /* Determine a value below which we try to not lower the exposure,
-  as most exposure controls tend to jump with big steps in the low
-  range, causing oscilation, so we prefer to use gain when exposure
-  has hit this value */
-   exposure_low = (expoctrl.maximum - expoctrl.minimum) / 10;
-   /* If we have a fine grained exposure control only avoid the last 10 
steps */
-   steps = exposure_low / expoctrl.step;
-   if (steps > 10)
-   steps = 10;
-   exposure_low = steps * expoctrl.step + expoctrl.minimum;
-
-   ctrl.id = V4L2_CID_GAIN;
-   gainctrl.id = V4L2_CID_GAIN;
-   if (SYS_IOCTL(data->fd, VIDIOC_QUERYCTRL, ) ||
-   SYS_IOCTL(data->fd, VIDIOC_G_CTRL, ))
-   return 0;
-   gain = orig_gain = ctrl.value;
-
-   switch (fmt->fmt.pix.pixelformat) {
-   case V4L2_PIX_FMT_SGBRG8:
-   case V4L2_PIX_FMT_SGRBG8:
-   case V4L2_PIX_FMT_SBGGR8:
-   case V4L2_PIX_FMT_SRGGB8:
-   buf += fmt->fmt.pix.height * fmt->fmt.pix.bytesperline / 4 +
-   fmt->fmt.pix.width / 4;
-
-   for (y = 0; y < fmt->fmt.pix.height / 2; y++) {
-   for (x = 0; x < fmt->fmt.pix.width / 2; x++)
-   avg_lum += *buf++;
-   buf += fmt->fmt.pix.bytesperline - fmt->fmt.pix.width / 
2;
-   }
-   avg_lum /= fmt->fmt.pix.height * fmt->fmt.pix.width / 4;
-

et8ek8: Document support for flash and lens devices

[Pavel Machek]

Document dts support of LED/focus.

Signed-off-by: Pavel Machek <pa...@ucw.cz>

diff --git a/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt 
b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
index 0b7b6a4..e80d589 100644
--- a/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
+++ b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
@@ -20,6 +20,13 @@ Mandatory properties
   is in hardware standby mode when the signal is in the low state.
 
 
+Optional properties
+---
+
+- flash-leds: See ../video-interfaces.txt
+- lens-focus: See ../video-interfaces.txt
+
+
 Endpoint node mandatory properties
 --
 
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: Nokia N9: fun with camera

[Pavel Machek]

Hi!

> > Sakari, I am actually playing with N9 camera, not N950. That comes
> > next.
> > 
> > And the clock error I mentioned ... seems to be
> > -EPROBE_DEFER. So... not an issue.
> 
> Hmm, and with similar config, I got N950 to work. ... which should
> give me enough clues to get N9 to work. I guess I forgot to reset the
> pipeline between the tries, or something.
> 
> For the record, this got me some data on n950:
> 
>  m.media_ctl( [ '-f', '"OMAP3 ISP CSI2a":0 [fmt:%s/%dx%d]' % (m.fmt, m.cap_x, 
> m.cap_y) ] )
>  m.media_ctl( [ '-l', '"OMAP3 ISP CSI2a":1 -> "OMAP3 ISP CSI2a output":0[1]' 
> ] )
> 
>  # WORKS
>  # pavel@n900:~/g/tui/camera$ sudo /my/tui/yavta/yavta
>  # --capture=8 --skip 0 --format SGRBG10 --size 4272x3016 /dev/video1 
> --file=/tmp/delme#
> 
> ...ouch. It only worked twice :-(. Either driver gets confused by my
> attempts, or it relied on some other initialization code. Strange.

Hmm, so it works "reliably" after boot. But it also locks up machine
with high probability. If you have N950, it might still be handy...

Messages are:

['-r']
['-f', '"OMAP3 ISP CSI2a":0 [fmt:SGRBG10/4272x3016]']
Warning: the -f option is deprecated and has been replaced by -V.
['-l', '"OMAP3 ISP CSI2a":1 -> "OMAP3 ISP CSI2a output":0[1]']
Testing: is raw
Testing:  /my/tui/yavta/yavta --capture=8 --skip 0 --format SGRBG10
--size 4272x3016 /dev/video_sensor --file=/tmp/delme#
Error opening device /dev/video_sensor: Permission denied (13).
Raw mode is  1
Raw mode is  1
Testing: is raw
Testing:  /my/tui/yavta/yavta --capture=8 --skip 0 --format SGRBG10
--size 4272x3016 /dev/video_sensor --file=/tmp/delme#
Device /dev/video_sensor opened.
Device `OMAP3 ISP CSI2a output' on `media' is a video capture (without
mplanes) device.
Video format set: SGRBG10 (30314142) 4272x3016 (stride 8544) field
none buffer size 25768704
Video format: SGRBG10 (30314142) 4272x3016 (stride 8544) field none
buffer size 25768704
1 buffers requested.
length: 25768704 offset: 0 timestamp type/source: mono/EoF
Buffer 0/0 mapped at address 0xb54e9000.
0 (0) [-] none 0 25768704 B 691.756907 691.758403 6.212 fps ts
mono/EoF
1 (0) [-] none 1 25768704 B 694.821025 694.821422 0.326 fps ts
mono/EoF
2 (0) [E] none 2 25768704 B 698.530833 698.534739 0.270 fps ts
mono/EoF
3 (0) [-] none 3 25768704 B 700.788127 700.790996 0.443 fps ts
mono/EoF
(and here it locked up :-(. Sometimes it captures more.)

Thanks,

Pavel




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


signature.asc
Description: Digital signature

Re: Nokia N9: fun with camera

[Pavel Machek]

Hi!

> Sakari, I am actually playing with N9 camera, not N950. That comes
> next.
> 
> And the clock error I mentioned ... seems to be
> -EPROBE_DEFER. So... not an issue.

Hmm, and with similar config, I got N950 to work. ... which should
give me enough clues to get N9 to work. I guess I forgot to reset the
pipeline between the tries, or something.

For the record, this got me some data on n950:

 m.media_ctl( [ '-f', '"OMAP3 ISP CSI2a":0 [fmt:%s/%dx%d]' % (m.fmt, m.cap_x, 
m.cap_y) ] )
 m.media_ctl( [ '-l', '"OMAP3 ISP CSI2a":1 -> "OMAP3 ISP CSI2a output":0[1]' ] )

 # WORKS
 # pavel@n900:~/g/tui/camera$ sudo /my/tui/yavta/yavta
 # --capture=8 --skip 0 --format SGRBG10 --size 4272x3016 /dev/video1 
--file=/tmp/delme#

...ouch. It only worked twice :-(. Either driver gets confused by my
attempts, or it relied on some other initialization code. Strange.

Best regards,
Pavel

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


signature.asc
Description: Digital signature

Nokia N9: fun with camera

[Pavel Machek]

Hi!

Sakari, I am actually playing with N9 camera, not N950. That comes
next.

And the clock error I mentioned ... seems to be
-EPROBE_DEFER. So... not an issue.

Strange thing is, that my sensors seems to have different resolution
from yours:

- entity 89: smiapp pixel_array 1-0010 (1 pad, 1 link)
 type V4L2 subdev subtype Sensor flags 0
  device node name /dev/v4l-subdev9
  pad0: Source [fmt:SRGGB10_1X10/3572x2464 field:none
  crop.bounds:(0,0)/3572x2464 crop:(0,0)/3572x2464]
 ->
 "smiapp binner 1-0010":0 [ENABLED,IMMUTABLE]

(And you mentioned width 3600, and SGRBG10).

I updated my scripts accordingly. Now I get

pavel@n900:~/g/tui/camera$ /my/tui/yavta/yavta -c5 -f SRGGB10
-F/tmp/foo -s 3572x2464 /dev/video1
Device /dev/video1 opened.
Device `OMAP3 ISP CSI2a output' on `media' is a video capture (without
mplanes) device.
Video format set: SRGGB10 (30314752) 3572x2464 (stride 7144) field
none buffer size 17602816
Video format: SRGGB10 (30314752) 3572x2464 (stride 7144) field none
buffer size 17602816
2 buffers requested.
length: 17602816 offset: 0 timestamp type/source: mono/EoF
Buffer 0/0 mapped at address 0xb5cb1000.
length: 17602816 offset: 17604608 timestamp type/source: mono/EoF
Buffer 1/0 mapped at address 0xb4be7000.

...but here it hangs. (Kernel v4.13).

dmesg says:

[ 2862.229736] smiapp 1-0010: flip 0
[ 2862.229766] smiapp 1-0010: new pixel order RGGB
[ 2862.233764] smiapp 1-0010: 0x00021700 "min_frame_length_lines_bin"
= 166, 0xa6
[ 2862.234558] smiapp 1-0010: 0x00021702 "max_frame_length_lines_bin"
= 65535, 0x
[ 2862.235351] smiapp 1-0010: 0x00021704 "min_line_length_pck_bin" =
3812, 0xee4
[ 2862.236114] smiapp 1-0010: 0x00021706 "max_line_length_pck_bin" =
32752, 0x7ff0
[ 2862.238067] smiapp 1-0010: 0x00021708 "min_line_blanking_pck_bin" =
240, 0xf0
[ 2862.240844] smiapp 1-0010: 0x0002170a
"fine_integration_time_min_bin" = 0, 0x0
[ 2862.242553] smiapp 1-0010: 0x0002170c
"fine_integration_time_max_margin_bin" = 0, 0x0
[ 2862.242614] smiapp 1-0010: vblank 26
[ 2862.242645] smiapp 1-0010: hblank240
[ 2862.242675] smiapp 1-0010: real timeperframe 100/839

I did some more experiments, but could not grab a frame.

Best regards,
Pavel


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


signature.asc
Description: Digital signature

Re: [patch] libv4l2: SDL test application

[Pavel Machek]

On Mon 2017-10-30 17:30:53, Hans Verkuil wrote:
> Hi Pavel,
> 
> On 10/28/2017 09:57 PM, Pavel Machek wrote:
> > Add support for simple SDL test application. Allows taking jpeg
> > snapshots, and is meant to run on phone with touchscreen. Not
> > particulary useful on PC with webcam, but should work.
> 
> When I try to build this I get:
> 
> make[3]: Entering directory '/home/hans/work/src/v4l/v4l-utils/contrib/test'
>   CCLD sdlcam
> /usr/bin/ld: sdlcam-sdlcam.o: undefined reference to symbol 
> 'log2@@GLIBC_2.2.5'
> //lib/x86_64-linux-gnu/libm.so.6: error adding symbols: DSO missing from 
> command line
> collect2: error: ld returned 1 exit status
> Makefile:561: recipe for target 'sdlcam' failed
> make[3]: *** [sdlcam] Error 1
> make[3]: Leaving directory '/home/hans/work/src/v4l/v4l-utils/contrib/test'
> Makefile:475: recipe for target 'all-recursive' failed
> make[2]: *** [all-recursive] Error 1
> make[2]: Leaving directory '/home/hans/work/src/v4l/v4l-utils/contrib'
> Makefile:589: recipe for target 'all-recursive' failed
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory '/home/hans/work/src/v4l/v4l-utils'
> Makefile:516: recipe for target 'all' failed
> make: *** [all] Error 2
> 
> I had to add -lm -ldl -lrt to sdlcam_LDFLAGS. Is that correct?

Yes, that should be correct. I had that problem, too, but I thought I
solved it with simpler configure.ac.

Best regards,
Pavel

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


signature.asc
Description: Digital signature

[patch] libv4l2: SDL test application

[Pavel Machek]

Add support for simple SDL test application. Allows taking jpeg
snapshots, and is meant to run on phone with touchscreen. Not
particulary useful on PC with webcam, but should work.

Signed-off-by: Pavel Machek <pa...@ucw.cz>

diff --git a/configure.ac b/configure.ac
index f3691be..f6540c2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -439,6 +439,9 @@ AC_ARG_ENABLE(gconv,
esac]
 )
 
+PKG_CHECK_MODULES([SDL2], [sdl2 SDL2_image], [sdl_pc=yes], [sdl_pc=no])
+AM_CONDITIONAL([HAVE_SDL], [test x$sdl_pc = xyes])
+
 # Check if backtrace functions are defined
 AC_SEARCH_LIBS([backtrace], [execinfo], [
   AC_DEFINE(HAVE_BACKTRACE, [1], [glibc has functions to provide stack 
backtrace])
@@ -507,6 +510,7 @@ compile time options summary
 pthread: $have_pthread
 QT version : $QT_VERSION
 ALSA support   : $USE_ALSA
+SDL support   : $sdl_pc
 
 build dynamic libs : $enable_shared
 build static libs  : $enable_static
diff --git a/contrib/test/Makefile.am b/contrib/test/Makefile.am
index 4641e21..0f97ce2 100644
--- a/contrib/test/Makefile.am
+++ b/contrib/test/Makefile.am
@@ -16,6 +16,10 @@ if HAVE_GLU
 noinst_PROGRAMS += v4l2gl
 endif
 
+if HAVE_SDL
+noinst_PROGRAMS += sdlcam
+endif
+
 driver_test_SOURCES = driver-test.c
 driver_test_LDADD = ../../utils/libv4l2util/libv4l2util.la
 
@@ -31,6 +35,10 @@ v4l2gl_SOURCES = v4l2gl.c
 v4l2gl_LDFLAGS = $(X11_LIBS) $(GL_LIBS) $(GLU_LIBS) $(ARGP_LIBS)
 v4l2gl_LDADD = ../../lib/libv4l2/libv4l2.la 
../../lib/libv4lconvert/libv4lconvert.la
 
+sdlcam_LDFLAGS = $(JPEG_LIBS) $(SDL2_LIBS)
+sdlcam_CFLAGS = -I../.. $(SDL2_CFLAGS)
+sdlcam_LDADD = ../../lib/libv4l2/.libs/libv4l2.a  
../../lib/libv4lconvert/.libs/libv4lconvert.a
+
 mc_nextgen_test_CFLAGS = $(LIBUDEV_CFLAGS)
 mc_nextgen_test_LDFLAGS = $(LIBUDEV_LIBS)
 
diff --git a/contrib/test/sdlcam.c b/contrib/test/sdlcam.c
new file mode 100644
index 000..cc43a10
--- /dev/null
+++ b/contrib/test/sdlcam.c
@@ -0,0 +1,1250 @@
+/*
+   Digital still camera.
+
+   SDL based, suitable for camera phone such as Nokia N900. In
+   particular, we support focus, gain and exposure control, but not
+   aperture control or lens zoom.
+
+   Copyright 2017 Pavel Machek, LGPL
+
+   Needs sdl2, sdl2_image libraries. sudo aptitude install libsdl2-dev
+   libsdl2-image-dev on Debian systems.
+*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "libv4l2.h"
+#include 
+#include "libv4l-plugin.h"
+
+#include 
+#include 
+
+#define PICDIR "."
+#define SX 2592
+#define SY 1968
+#define SIZE SX*SY*3
+
+static void fmt_print(struct v4l2_format *fmt)
+{
+   int f;
+   printf("Format: %dx%d. ", fmt->fmt.pix.width, fmt->fmt.pix.height);
+   printf("%x ", fmt->fmt.pix.pixelformat);
+   f = fmt->fmt.pix.pixelformat;
+   for (int i=0; i<4; i++) {
+   printf("%c", f & 0xff);
+   f >>= 8;
+   }
+   printf("\n");
+}
+
+static double dtime(void)
+{
+   static double start = 0.0;
+   struct timeval now;
+
+   gettimeofday(, NULL);
+
+   double n = now.tv_sec + now.tv_usec / 100.;
+   if (!start)
+   start = n;
+   return n - start;
+}
+
+static long v4l2_g_ctrl(int fd, long id)
+{
+   int res;
+   struct v4l2_control ctrl;
+   ctrl.id = id;
+   ctrl.value = 0;
+   res = v4l2_ioctl(fd, VIDIOC_G_CTRL, );
+   if (res < 0)
+   printf("Get control %lx failed\n", id);
+   return ctrl.value;
+}
+
+static int v4l2_s_ctrl(int fd, long id, long value)
+{
+   int res;
+   struct v4l2_control ctrl;
+   ctrl.id = id;
+   ctrl.value = value;
+   res = v4l2_ioctl(fd, VIDIOC_S_CTRL, );
+   if (res < 0)
+   printf("Set control %lx %ld failed\n", id, value);
+   return res;
+}
+
+static int v4l2_set_focus(int fd, int diopt)
+{
+   if (v4l2_s_ctrl(fd, V4L2_CID_FOCUS_ABSOLUTE, diopt) < 0) {
+   printf("Could not set focus\n");
+   }
+   return 0;
+}
+
+struct dev_info {
+   int fd;
+   struct v4l2_format fmt;
+
+   unsigned char buf[SIZE];
+   int debug;
+#define D_TIMING 1
+};
+
+struct sdl {
+   SDL_Window *window;
+   SDL_Surface *screen, *liveview;
+
+   int wx, wy; /* Window size */
+   int sx, sy; /* Live view size */
+   int bx, by; /* Border size */
+   int nx, ny; /* Number of buttons */
+   float factor;
+
+   /* These should go separately */
+   int do_focus, do_exposure, do_flash, do_white, do_big, do_full;
+   double zoom;
+   double focus_min;
+
+   int slider_mode;
+#define M_BIAS 0
+#define M_EXPOSURE 1
+#define M_GAIN 2
+#define M

Re: Camera support, Prague next week, sdlcam

[Pavel Machek]

Hi!

> > I'll talk about the issues at ELCE in few days:
> > 
> > https://osseu17.sched.com/event/ByYH/cheap-complex-cameras-pavel-machek-denx-software-engineering-gmbh
> > 
> > Will someone else be there? Is there some place where v4l people meet?
> 
> Why don't we discuss this Tuesday morning at 9am? I have no interest in the
> keynotes on that day, so those who are interested can get together.

Ok, what about 9:30am? The place near the elevators where we were
talking last time?

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


signature.asc
Description: Digital signature

Re: Camera support, Prague next week, sdlcam

[Pavel Machek]

Hi!

> > I'd still like to get some reasonable support for cellphone camera in
> > Linux.
> > 
> > IMO first reasonable step is to merge sdlcam, then we can implement
> > autofocus, improve autogain... and rest of the boring stuff. Ouch and
> > media graph support would be nice. Currently, _nothing_ works with
> > media graph device, such as N900.
> 
> Can you post your latest rebased patch for sdlcam for v4l-utils?
> 
> I'll do a review and will likely merge it for you. Yes, I've changed my
> mind on that.

Ok, will do, thanks!

> > I'll talk about the issues at ELCE in few days:
> > 
> > https://osseu17.sched.com/event/ByYH/cheap-complex-cameras-pavel-machek-denx-software-engineering-gmbh
> > 
> > Will someone else be there? Is there some place where v4l people meet?
> 
> Why don't we discuss this Tuesday morning at 9am? I have no interest in the
> keynotes on that day, so those who are interested can get together.
> 
> I'll be at your presentation tomorrow and we can discuss a bit during
> the following coffee break if time permits.

Ok, sounds like a plan. Lets confirm Tuesday morning tommorow.

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


signature.asc
Description: Digital signature

Camera support, Prague next week, sdlcam

[Pavel Machek]

Hi!

I'd still like to get some reasonable support for cellphone camera in
Linux.

IMO first reasonable step is to merge sdlcam, then we can implement
autofocus, improve autogain... and rest of the boring stuff. Ouch and
media graph support would be nice. Currently, _nothing_ works with
media graph device, such as N900.

I'll talk about the issues at ELCE in few days:

https://osseu17.sched.com/event/ByYH/cheap-complex-cameras-pavel-machek-denx-software-engineering-gmbh

Will someone else be there? Is there some place where v4l people meet?

Best regards,

Pavel

commit 8db1546cd0b5229ad7bf2605fd5c295365df57f8
Author: Pavel <pa...@ucw.cz>
Date:   Fri Oct 13 21:24:16 2017 +0200

Port changes from "cleaner" branch. For some reason, SDL2_image is
missing in compilation.

gcc -std=gnu99 -I../.. -g -O2 -o sdlcam sdlcam-sdlcam.o  -ljpeg -lSDL2
-lSDL2_image ../../lib/libv4l2/.libs/libv4l2.a 
../../lib/libv4lconvert/.libs/libv4lconvert.a

diff --git a/configure.ac b/configure.ac
index f3691be..dc412c9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -98,6 +98,247 @@ LIBDVBV5_DOMAIN="libdvbv5"
 AC_DEFINE([LIBDVBV5_DOMAIN], "libdvbv5", [libdvbv5 domain])
 AC_SUBST(LIBDVBV5_DOMAIN)
 
+# Configure paths for SDL
+# Sam Lantinga 9/21/99
+# stolen from Manish Singh
+# stolen back from Frank Belew
+# stolen from Manish Singh
+# Shamelessly stolen from Owen Taylor
+#
+# Changelog:
+# * also look for SDL2.framework under Mac OS X
+
+# serial 1
+
+dnl AM_PATH_SDL2([MINIMUM-VERSION, [ACTION-IF-FOUND [, ACTION-IF-NOT-FOUND]]])
+dnl Test for SDL, and define SDL_CFLAGS and SDL_LIBS
+dnl
+AC_DEFUN([AM_PATH_SDL2],
+[dnl 
+dnl Get the cflags and libraries from the sdl2-config script
+dnl
+AC_ARG_WITH(sdl-prefix,[  --with-sdl-prefix=PFX   Prefix where SDL is 
installed (optional)],
+sdl_prefix="$withval", sdl_prefix="")
+AC_ARG_WITH(sdl-exec-prefix,[  --with-sdl-exec-prefix=PFX Exec prefix where 
SDL is installed (optional)],
+sdl_exec_prefix="$withval", sdl_exec_prefix="")
+AC_ARG_ENABLE(sdltest, [  --disable-sdltest   Do not try to compile and 
run a test SDL program],
+   , enable_sdltest=yes)
+AC_ARG_ENABLE(sdlframework, [  --disable-sdlframework Do not search for 
SDL2.framework],
+, search_sdl_framework=yes)
+
+AC_ARG_VAR(SDL2_FRAMEWORK, [Path to SDL2.framework])
+
+  min_sdl_version=ifelse([$1], ,2.0.0,$1)
+
+  if test "x$sdl_prefix$sdl_exec_prefix" = x ; then
+PKG_CHECK_MODULES([SDL], [sdl2 >= $min_sdl_version],
+   [sdl_pc=yes],
+   [sdl_pc=no])
+  else
+sdl_pc=no
+if test x$sdl_exec_prefix != x ; then
+  sdl_config_args="$sdl_config_args --exec-prefix=$sdl_exec_prefix"
+  if test x${SDL2_CONFIG+set} != xset ; then
+SDL2_CONFIG=$sdl_exec_prefix/bin/sdl2-config
+  fi
+fi
+if test x$sdl_prefix != x ; then
+  sdl_config_args="$sdl_config_args --prefix=$sdl_prefix"
+  if test x${SDL2_CONFIG+set} != xset ; then
+SDL2_CONFIG=$sdl_prefix/bin/sdl2-config
+  fi
+fi
+  fi
+
+  if test "x$sdl_pc" = xyes ; then
+no_sdl=""
+SDL2_CONFIG="pkg-config sdl2"
+  else
+as_save_PATH="$PATH"
+if test "x$prefix" != xNONE && test "$cross_compiling" != yes; then
+  PATH="$prefix/bin:$prefix/usr/bin:$PATH"
+fi
+AC_PATH_PROG(SDL2_CONFIG, sdl2-config, no, [$PATH])
+PATH="$as_save_PATH"
+no_sdl=""
+
+if test "$SDL2_CONFIG" = "no" -a "x$search_sdl_framework" = "xyes"; then
+  AC_MSG_CHECKING(for SDL2.framework)
+  if test "x$SDL2_FRAMEWORK" != x; then
+sdl_framework=$SDL2_FRAMEWORK
+  else
+for d in / ~/ /System/; do
+  if test -d "$dLibrary/Frameworks/SDL2.framework"; then
+sdl_framework="$dLibrary/Frameworks/SDL2.framework"
+  fi
+done
+  fi
+
+  if test -d $sdl_framework; then
+AC_MSG_RESULT($sdl_framework)
+sdl_framework_dir=`dirname $sdl_framework`
+SDL_CFLAGS="-F$sdl_framework_dir -Wl,-framework,SDL2 
-I$sdl_framework/include"
+SDL_LIBS="-F$sdl_framework_dir -Wl,-framework,SDL2"
+  else
+no_sdl=yes
+  fi
+fi
+
+if test "$SDL2_CONFIG" != "no"; then
+  if test "x$sdl_pc" = "xno"; then
+AC_MSG_CHECKING(for SDL - version >= $min_sdl_version)
+SDL_CFLAGS=`$SDL2_CONFIG $sdl_config_args --cflags`
+SDL_LIBS=`$SDL2_CONFIG $sdl_config_args --libs`
+  fi
+
+  sdl_major_version=`$SDL2_CONFIG $sdl_config_args --version | \
+ sed 's/\([[0-9]]*\).\([[0-9]]*\).\([[0-9]]*\)/\1/'`
+  sdl_min

Re: [PATCH 04/24] media: v4l2-mediabus: convert flags to enums and document them

[Pavel Machek]

On Mon 2017-10-09 07:19:10, Mauro Carvalho Chehab wrote:
> There is a mess with media bus flags: there are two sets of
> flags, one used by parallel and ITU-R BT.656 outputs,
> and another one for CSI2.
> 
> Depending on the type, the same bit has different meanings.
> 

> @@ -86,11 +125,22 @@ enum v4l2_mbus_type {
>  /**
>   * struct v4l2_mbus_config - media bus configuration
>   * @type:in: interface type
> - * @flags:   in / out: configuration flags, depending on @type
> + * @pb_flags:in / out: configuration flags, if @type is
> + *   %V4L2_MBUS_PARALLEL or %V4L2_MBUS_BT656.
> + * @csi2_flags:  in / out: configuration flags, if @type is
> + *   %V4L2_MBUS_CSI2.
> + * @flag:access flags, no matter the @type.
> + *   Used just to avoid needing to rewrite the logic inside
> + *   soc_camera and pxa_camera drivers. Don't use on newer
> + *   drivers!
>   */
>  struct v4l2_mbus_config {
>   enum v4l2_mbus_type type;
> - unsigned int flags;
> + union {
> + enum v4l2_mbus_parallel_and_bt656_flags pb_flags;
> + enum v4l2_mbus_csi2_flags   csi2_flags;
> + unsigned intflag;
> + };
>  };
>  
>  static inline void v4l2_fill_pix_format(struct v4l2_pix_format
>  *pix_fmt,

The flags->flag conversion is quite subtle, and "flag" is confusing
because there is more than one inside. What about something like
__legacy_flags?

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


signature.asc
Description: Digital signature

Re: [PATCH v15 29/32] et8ek8: Add support for flash and lens devices

[Pavel Machek]

On Thu 2017-10-05 00:50:48, Sakari Ailus wrote:
> Parse async sub-devices related to the sensor by switching the async
> sub-device registration function.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Acked-by: Hans Verkuil <hans.verk...@cisco.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v15 28/32] smiapp: Add support for flash and lens devices

[Pavel Machek]

On Thu 2017-10-05 00:50:47, Sakari Ailus wrote:
> Parse async sub-devices related to the sensor by switching the async
> sub-device registration function.
> 
> These types devices aren't directly related to the sensor, but are
> nevertheless handled by the smiapp driver due to the relationship of these
> component to the main part of the camera module --- the sensor.
> 
> This does not yet address providing the user space with information on how
> to associate the sensor or lens devices but the kernel now has the
> necessary information to do that.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Acked-by: Hans Verkuil <hans.verk...@cisco.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [RESEND PATCH v2 4/6] dt: bindings: as3645a: Improve label documentation, DT example

[Pavel Machek]

On Mon 2017-09-18 17:49:23, Sakari Ailus wrote:
> Hi Pavel,
> 
> On Mon, Sep 18, 2017 at 12:56:55PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > Specify the exact label used if the label property is omitted in DT, as
> > > well as use label in the example that conforms to LED device naming.
> > > 
> > > @@ -69,11 +73,11 @@ Example
> > >   flash-max-microamp = <32>;
> > >   led-max-microamp = <6>;
> > >   ams,input-max-microamp = <175>;
> > > - label = "as3645a:flash";
> > > + label = "as3645a:white:flash";
> > >   };
> > >   indicator@1 {
> > >   reg = <0x1>;
> > >   led-max-microamp = <1>;
> > > - label = "as3645a:indicator";
> > > + label = "as3645a:red:indicator";
> > >   };
> > >   };
> > 
> > Ok, but userspace still has no chance to determine if this is flash
> > from main camera or flash for front camera; todays smartphones have
> > flashes on both cameras.
> > 
> > So.. Can I suggset as3645a:white:main_camera_flash or main_flash or
> > ?
> 
> If there's just a single one in the device, could you use that?
> 
> Even if we name this so for N9 (and N900), the application still would only
> work with the two devices.

Well, I'd plan to name it on other devices, too.

> My suggestion would be to look for a flash LED, and perhaps the maximum
> current as well. That should generally work better than assumptions on the
> label.

If you just look for flash LED, you don't know if it is front one or
back one. Its true that if you have just one flash it is usually on
the back camera, but you can't know if maybe driver is not available
for the main flash.

Lets get this right, please "main_camera_flash" is 12 bytes more than
"flash", and it saves application logic.. more than 12 bytes, I'm sure. 

> For association with a particular camera --- in the long run I'd propose to
> use Media controller / property API for that in the long run. We don't have
> that yet though.

We don't have that yet. Plus simple applications may not want to talk
v4l2 ioctls

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


signature.asc
Description: Digital signature

Re: [RESEND PATCH v2 4/6] dt: bindings: as3645a: Improve label documentation, DT example

[Pavel Machek]

Hi!

> Specify the exact label used if the label property is omitted in DT, as
> well as use label in the example that conforms to LED device naming.
> 
> @@ -69,11 +73,11 @@ Example
>   flash-max-microamp = <32>;
>   led-max-microamp = <6>;
>   ams,input-max-microamp = <175>;
> - label = "as3645a:flash";
> + label = "as3645a:white:flash";
>   };
>   indicator@1 {
>   reg = <0x1>;
>   led-max-microamp = <1>;
> - label = "as3645a:indicator";
> + label = "as3645a:red:indicator";
>   };
>   };

Ok, but userspace still has no chance to determine if this is flash
from main camera or flash for front camera; todays smartphones have
flashes on both cameras.

So.. Can I suggset as3645a:white:main_camera_flash or main_flash or
?

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


signature.asc
Description: Digital signature

Re: [PATCH v13 06/25] omap3isp: Use generic parser for parsing fwnode endpoints

[Pavel Machek]

On Sat 2017-09-16 09:04:31, Pavel Machek wrote:
> Hi!
> 
> > Instead of using driver implementation, use
> > v4l2_async_notifier_parse_fwnode_endpoints() to parse the fwnode endpoints
> > of the device.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> > Acked-by: Hans Verkuil <hans.verk...@cisco.com>
> 
> Patches at least up to here look fine o me.

I went through some others.

So, 2,4,5,6,10,12:

Acked-by: Pavel Machek <pa...@ucw.cz>

Best regards,


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


signature.asc
Description: Digital signature

Re: [PATCH v13 11/25] v4l: async: Introduce helpers for calling async ops callbacks

[Pavel Machek]

On Fri 2017-09-15 17:17:10, Sakari Ailus wrote:
> Add three helper functions to call async operations callbacks. Besides
> simplifying callbacks, this allows async notifiers to have no ops set,
> i.e. it can be left NULL.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Acked-by: Hans Verkuil <hans.verk...@cisco.com>

I'd remove "_call" from these names; they are long enough already and
do not add much. But either way is okay.

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v13 06/25] omap3isp: Use generic parser for parsing fwnode endpoints

[Pavel Machek]

Hi!

> Instead of using driver implementation, use
> v4l2_async_notifier_parse_fwnode_endpoints() to parse the fwnode endpoints
> of the device.
> 
> Signed-off-by: Sakari Ailus 
> Acked-by: Hans Verkuil 

Patches at least up to here look fine o me.

We are at version 13 of the series... Is merge of the series expected
anytimme soon?

If not, can we at least merge patches up to here, so that less stuff
is retransmitted over and over?

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


signature.asc
Description: Digital signature

Re: as3645a flash userland interface

[Pavel Machek]

On Thu 2017-09-14 13:01:19, Sylwester Nawrocki wrote:
> On 09/14/2017 12:07 PM, Pavel Machek wrote:
> >>Isn't the V4L2 subdev/Media Controller API supposed to provide means
> >>for associating flash LEDs with camera sensors? You seem to be insisting
> >>on using the sysfs leds interface for that, which is not a primary
> >>interface for camera flash AFAICT.
> >
> >a) subdev/media controller API currently does not provide such means.
> 
> Yes, but it should, that's what it was designed for AFAIK.
> 
> >b) if we have /sys/class/leds interface to userland, it should be
> >useful.
> 
> At the same time we shouldn't overcomplicate it with the camera
> functionality.

I'm advocating adding label = "main_camera" into the .dts. That's all.

> >c) having flashlight application going through media controller API is
> >a bad joke.
> 
> It doesn't have to, maybe I misunderstood what you exactly ask for.
> Nevertheless what's missing is some user visible name/label for each
> flash LED, right? Currently enumerating flash LEDs can be done by looking
> at the function part of /sys/class/leds/::
>  path.
> 
> Could additional information be appended to the  part, so
> user can identify which LED is which? E.g. "flash(rear)", "flash(front)",
> etc. This could be achieved by simply adding label property in DT.
> Or is the list of supported  strings already standardized?

label = "flash_main_camera" would work for me, yes. And yes, I'd
prefer to do this before 4.14 release, so that userland-visible
interface does not change.

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


signature.asc
Description: Digital signature

Re: as3645a flash userland interface

[Pavel Machek]

Hi!

> What directory are the flash controls in?
> 
> /sys/class/leds/led-controller:flash ?
> 
> Could we arrange for something less generic, like
> 
> /sys/class/leds/main-camera:flash ?
> >>>
> >>>I'd rather avoid overcomplicating this. LED class device name pattern
> >>>is well defined to devicename:colour:function
> >>>(see Documentation/leds/leds-class.txt, "LED Device Naming" section).
> >>>
> >>>In this case "flash" in place of the "function" segment makes the
> >>>things clear enough I suppose.
> >>
> >>It does not.
> >>
> >>Phones usually have two cameras, front and back, and these days both
> >>cameras have their flash.
> >>
> >>And poor userspace flashlight application can not know if as3645
> >>drivers front LED or back LED. Thus, I'd set devicename to
> >>front-camera or main-camera -- because that's what it is associated
> >>with. Userspace does not care what hardware drives the LED, but needs
> >>to know if it is front or back camera.
> >
> >The name of a LED flash class device isn't fixed and is derived
> >from DT label property. Name in the example of some DT bindings
> >will not force people to apply similar pattern for the other
> >drivers and even for the related one. No worry about having
> >to keep anything forever basing on that.
> 
> Isn't the V4L2 subdev/Media Controller API supposed to provide means
> for associating flash LEDs with camera sensors? You seem to be insisting
> on using the sysfs leds interface for that, which is not a primary
> interface for camera flash AFAICT.

a) subdev/media controller API currently does not provide such means.

b) if we have /sys/class/leds interface to userland, it should be
useful.

c) having flashlight application going through media controller API is
a bad joke.

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


signature.asc
Description: Digital signature

Re: as3645a flash userland interface

[Pavel Machek]

On Tue 2017-09-12 20:53:33, Jacek Anaszewski wrote:
> Hi Pavel,
> 
> On 09/12/2017 12:36 PM, Pavel Machek wrote:
> > Hi!
> > 
> > There were some changes to as3645a flash controller. Before we have
> > stable interface we have to keep forever I want to ask:
> 
> Note that we have already two LED flash class drivers - leds-max77693
> and leds-aat1290. They have been present in mainline for over two years
> now.

Well.. that's ok. No change there is neccessary.

> > What directory are the flash controls in?
> > 
> > /sys/class/leds/led-controller:flash ?
> > 
> > Could we arrange for something less generic, like
> > 
> > /sys/class/leds/main-camera:flash ?
> 
> I'd rather avoid overcomplicating this. LED class device name pattern
> is well defined to devicename:colour:function
> (see Documentation/leds/leds-class.txt, "LED Device Naming" section).
> 
> In this case "flash" in place of the "function" segment makes the
> things clear enough I suppose.

It does not.

Phones usually have two cameras, front and back, and these days both
cameras have their flash.

And poor userspace flashlight application can not know if as3645
drivers front LED or back LED. Thus, I'd set devicename to
front-camera or main-camera -- because that's what it is associated
with. Userspace does not care what hardware drives the LED, but needs
to know if it is front or back camera.

If LEDs control keyboard backlight, I'd expect the LED name to be
"keyboard::backlight", not "i2c-0020-adp1643::backlight".

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


signature.asc
Description: Digital signature

Re: as3645a flash userland interface

[Pavel Machek]

Hi!

> On Tue, Sep 12, 2017 at 12:36:28PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > There were some changes to as3645a flash controller. Before we have
> > stable interface we have to keep forever I want to ask:
> > 
> > What directory are the flash controls in?
> > 
> > /sys/class/leds/led-controller:flash ?
> > 
> > Could we arrange for something less generic, like
> > 
> > /sys/class/leds/main-camera:flash ?
> > 
> > Thanks,
> 
> The LEDs are called as3645a:flash and as3645a:indicator currently, based on
> the name of the LED controller's device node. There are no patches related
> to this set though; these have already been merged.
> 
> The label should be a "human readable string describing the device" (from
> ePAPR, please excuse me for not having a newer spec), and the led common
> bindings define it as:
> 
> - label : The label for this LED. If omitted, the label is taken from the node
>   name (excluding the unit address). It has to uniquely identify
>   a device, i.e. no other LED class device can be assigned the same
>   label.

Ok, can we set the label to "main_camera" for N9 and n950 cases?

"as3645a:flash" is really wrong name for a LED. Information that
as3645 is already present elsewhere in /sys. Information where the LED
is and what it does is not.

I'd like to have torch application that just writes
/sys/class/leds/main_camera:white:flash/brightness . It should not
need to know hardware details of differnet phones.

> I don't think that you should be looking to use this to associate it with
> the camera as such. The association information with the sensor is
> available to the kernel but there's no interface that could meaningfully
> expose it to the user right now.

Yeah, I'm not looking for sensor association. I'm looking for
reasonable userland interface.

Thanks,
Pavel

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


signature.asc
Description: Digital signature

as3645a flash userland interface

[Pavel Machek]

Hi!

There were some changes to as3645a flash controller. Before we have
stable interface we have to keep forever I want to ask:

What directory are the flash controls in?

/sys/class/leds/led-controller:flash ?

Could we arrange for something less generic, like

/sys/class/leds/main-camera:flash ?

Thanks,

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


signature.asc
Description: Digital signature

Re: [PATCH v11 21/24] smiapp: Add support for flash and lens devices

[Pavel Machek]

On Tue 2017-09-12 11:42:33, Sakari Ailus wrote:
> Parse async sub-devices by using
> v4l2_subdev_fwnode_reference_parse_sensor_common().
> 
> These types devices aren't directly related to the sensor, but are
> nevertheless handled by the smiapp driver due to the relationship of these
> component to the main part of the camera module --- the sensor.
> 
> This does not yet address providing the user space with information on how
> to associate the sensor or lens devices but the kernel now has the
> necessary information to do that.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Acked-by: Hans Verkuil <hans.verk...@cisco.com>

Acked-by: Pavel Machek <pa...@ucw.cz>


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


signature.asc
Description: Digital signature

[PATCH] et8ek8: Add support for flash and lens devices

[Pavel Machek]

Parse async sub-devices by using
v4l2_subdev_fwnode_reference_parse_sensor_common().

These types devices aren't directly related to the sensor, but are
nevertheless handled by the et8ek8 driver due to the relationship of these
component to the main part of the camera module --- the sensor.

Signed-off-by: Pavel Machek <pa...@ucw.cz>

---

This enables me to do autofocus on n900.

Depends on Sakari's series, so best solution would be to append it there.

Thanks,
Pavel

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c 
b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index c14f0fd..cd1f15f 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -34,10 +34,12 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #include "et8ek8_reg.h"
 
@@ -46,6 +48,7 @@
 #define ET8EK8_MAX_MSG 8
 
 struct et8ek8_sensor {
+   struct v4l2_async_notifier notifier;
struct v4l2_subdev subdev;
struct media_pad pad;
struct v4l2_mbus_framefmt format;
@@ -1446,6 +1449,11 @@ static int et8ek8_probe(struct i2c_client *client,
sensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
sensor->subdev.internal_ops = _internal_ops;
 
+   ret = v4l2_fwnode_reference_parse_sensor_common(
+   >dev, >notifier);
+   if (ret < 0 && ret != -ENOENT)
+   goto err_release;
+
sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
if (ret < 0) {
@@ -1453,18 +1461,27 @@ static int et8ek8_probe(struct i2c_client *client,
goto err_mutex;
}
 
+   ret = v4l2_async_subdev_notifier_register(>subdev,
+ >notifier);
+   if (ret)
+   goto err_entity;
+
ret = v4l2_async_register_subdev(>subdev);
if (ret < 0)
-   goto err_entity;
+   goto err_async;
 
dev_dbg(dev, "initialized!\n");
 
return 0;
 
+err_async:
+   v4l2_async_notifier_unregister(>notifier);
 err_entity:
media_entity_cleanup(>subdev.entity);
 err_mutex:
mutex_destroy(>power_lock);
+err_release:
+   v4l2_async_notifier_release(>notifier);
return ret;
 }
 
@@ -1480,6 +1497,8 @@ static int __exit et8ek8_remove(struct i2c_client *client)
}
 
v4l2_device_unregister_subdev(>subdev);
+   v4l2_async_notifier_unregister(>notifier);
+   v4l2_async_notifier_release(>notifier);
device_remove_file(>dev, _attr_priv_mem);
v4l2_ctrl_handler_free(>ctrl_handler);
v4l2_async_unregister_subdev(>subdev);

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


signature.asc
Description: Digital signature

Re: [PATCH v10 19/24] v4l: fwnode: Add convenience function for parsing common external refs

[Pavel Machek]

On Mon 2017-09-11 11:00:03, Sakari Ailus wrote:
> Add v4l2_fwnode_parse_reference_sensor_common for parsing common
> sensor properties that refer to adjacent devices such as flash or lens
> driver chips.
> 
> As this is an association only, there's little a regular driver needs to
> know about these devices as such.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Acked-by: Hans Verkuil <hans.verk...@cisco.com>
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 35 
> +++
>  include/media/v4l2-fwnode.h   | 13 +
>  2 files changed, 48 insertions(+)
> 
>  
> +/**
> + * v4l2_fwnode_reference_parse_sensor_common - parse common references on
> + *  sensors for async sub-devices
> + * @dev: the device node the properties of which are parsed for references
> + * @notifier: the async notifier where the async subdevs will be added
> + *
> + * Return: 0 on success
> + *  -ENOMEM if memory allocation failed
> + *  -EINVAL if property parsing failed
> + */

Returns: would sound more correct to me, but it seems kernel is split
on that.

Acked-by: Pavel Machek <pa...@ucw.cz>
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v10 24/24] arm: dts: omap3: N9/N950: Add flash references to the camera

[Pavel Machek]

On Mon 2017-09-11 11:00:08, Sakari Ailus wrote:
> Add flash and indicator LED phandles to the sensor node.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>

Acked-by: Pavel Machek <pa...@ucw.cz>


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


signature.asc
Description: Digital signature

Re: [PATCH v10 20/24] dt: bindings: smiapp: Document lens-focus and flash properties

[Pavel Machek]

On Mon 2017-09-11 11:00:04, Sakari Ailus wrote:
> Document optional lens-focus and flash properties for the smiapp
driver.

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

[RFC] et8ek8: Add support for flash and lens devices

[Pavel Machek]

Parse async sub-devices by using
v4l2_subdev_fwnode_reference_parse_sensor_common().

These types devices aren't directly related to the sensor, but are
nevertheless handled by the et8ek8 driver due to the relationship of these
component to the main part of the camera module --- the sensor.

Signed-off-by: Pavel Machek <pa...@ucw.cz> # Not yet ready -- broken whitespace

---

Whitespace is horribly bad. But otherwise... does it look ok?


diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c 
b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index c14f0fd..7714d2c 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -34,10 +34,12 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #include "et8ek8_reg.h"
 
@@ -46,6 +48,7 @@
 #define ET8EK8_MAX_MSG 8
 
 struct et8ek8_sensor {
+   struct v4l2_async_notifier notifier;
struct v4l2_subdev subdev;
struct media_pad pad;
struct v4l2_mbus_framefmt format;
@@ -1446,6 +1449,11 @@ static int et8ek8_probe(struct i2c_client *client,
sensor->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
sensor->subdev.internal_ops = _internal_ops;
 
+   ret = v4l2_fwnode_reference_parse_sensor_common(
+>dev, >notifier);
+if (ret < 0 && ret != -ENOENT)
+return ret;
+
sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(>subdev.entity, 1, >pad);
if (ret < 0) {
@@ -1453,14 +1461,21 @@ static int et8ek8_probe(struct i2c_client *client,
goto err_mutex;
}
 
+ret = v4l2_async_subdev_notifier_register(>subdev,
+ >notifier);
+   if (ret)
+   goto err_entity;
+
ret = v4l2_async_register_subdev(>subdev);
if (ret < 0)
-   goto err_entity;
+   goto err_async;
 
dev_dbg(dev, "initialized!\n");
 
return 0;
 
+err_async:
+   v4l2_async_notifier_unregister(>notifier);
 err_entity:
media_entity_cleanup(>subdev.entity);
 err_mutex:
@@ -1480,6 +1495,7 @@ static int __exit et8ek8_remove(struct i2c_client *client)
}
 
v4l2_device_unregister_subdev(>subdev);
+   v4l2_async_notifier_unregister(>notifier);
device_remove_file(>dev, _attr_priv_mem);
v4l2_ctrl_handler_free(>ctrl_handler);
v4l2_async_unregister_subdev(>subdev);


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


signature.asc
Description: Digital signature

Re: [PATCH v9 24/24] arm: dts: omap3: N9/N950: Add flash references to the camera

[Pavel Machek]

Hi!

> Add flash and indicator LED phandles to the sensor node.
> 
> Signed-off-by: Sakari Ailus 

I'm adding similar support to et8ek8 and wonder.. why don't you also
add support for autofocus? Driver not yet available?

Thanks,
Pavel
> @@ -26,6 +26,7 @@
>   clocks = < 0>;
>   clock-frequency = <960>;
>   nokia,nvm-size = <(16 * 64)>;
> + flash-leds = <_flash _indicator>;
>   port {
>   smia_1_1: endpoint {
>   link-frequencies = /bits/ 64 <19920 
> 21000 49920>;
> diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi 
> b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> index 1b0bd72945f2..12fbb3da5fce 100644
> --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
> +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> @@ -271,14 +271,14 @@
>   #size-cells = <0>;
>   reg = <0x30>;
>   compatible = "ams,as3645a";
> - flash@0 {
> + as3645a_flash: flash@0 {
>   reg = <0x0>;
>   flash-timeout-us = <15>;
>   flash-max-microamp = <32>;
>   led-max-microamp = <6>;
>   ams,input-max-microamp = <175>;
>   };
> - indicator@1 {
> + as3645a_indicator: indicator@1 {
>   reg = <0x1>;
>   led-max-microamp = <1>;
>   };
> diff --git a/arch/arm/boot/dts/omap3-n950.dts 
> b/arch/arm/boot/dts/omap3-n950.dts
> index 646601a3ebd8..c354a1ed1e70 100644
> --- a/arch/arm/boot/dts/omap3-n950.dts
> +++ b/arch/arm/boot/dts/omap3-n950.dts
> @@ -60,6 +60,7 @@
>   clocks = < 0>;
>   clock-frequency = <960>;
>   nokia,nvm-size = <(16 * 64)>;
> + flash-leds = <_flash _indicator>;
>   port {
>   smia_1_1: endpoint {
>   link-frequencies = /bits/ 64 <21000 
> 33360 39840>;

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


signature.asc
Description: Digital signature

Re: [PATCH v9 00/23] Unified fwnode endpoint parser, async sub-device notifier support, N9 flash DTS

[Pavel Machek]

On Fri 2017-09-08 16:25:07, Sakari Ailus wrote:
> On Fri, Sep 08, 2017 at 04:11:51PM +0300, Sakari Ailus wrote:
> > With this, the as3645a driver successfully registers a sub-device to the
> > media device created by the omap3isp driver. The kernel also has the
> > information it's related to the sensor driven by the smiapp driver but we
> > don't have a way to expose that information yet.
> 
> The patches are also available here:
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=fwnode-parse>

I merged the series on top of v4.14-rc0, and it does not break
anything. So:

Tested-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v9 18/24] as3645a: Switch to fwnode property API

[Pavel Machek]

On Fri 2017-09-08 16:18:16, Sakari Ailus wrote:
> Switch the as3645a from OF to the fwnode property API. Also add ACPI
> support.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v9 09/24] omap3isp: Print the name of the entity where no source pads could be found

[Pavel Machek]

On Fri 2017-09-08 16:18:07, Sakari Ailus wrote:
> If no source pads are found in an entity, print the name of the entity.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Acked-by: Hans Verkuil <hans.verk...@cisco.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v9 08/24] omap3isp: Fix check for our own sub-devices

[Pavel Machek]

On Fri 2017-09-08 16:18:06, Sakari Ailus wrote:
> We only want to link sub-devices that were bound to the async notifier the
> isp driver registered but there may be other sub-devices in the
> v4l2_device as well. Check for the correct async notifier.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Acked-by: Hans Verkuil <hans.verk...@cisco.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v9 03/24] v4l: async: Use more intuitive names for internal functions

[Pavel Machek]

On Fri 2017-09-08 16:11:54, Sakari Ailus wrote:
> Rename internal functions to make the names of the functions better
> describe what they do.
> 
>   Old nameNew name
>   v4l2_async_test_notify  v4l2_async_match_notify
>   v4l2_async_belongs  v4l2_async_find_match
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v9 15/24] dt: bindings: Add a binding for flash LED devices associated to a sensor

[Pavel Machek]

On Fri 2017-09-08 16:18:13, Sakari Ailus wrote:
> Camera flash drivers (and LEDs) are separate from the sensor devices in
> DT. In order to make an association between the two, provide the
> association information to the software.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Acked-by: Rob Herring <r...@kernel.org>
> Acked-by: Hans Verkuil <hans.verk...@cisco.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v9 17/24] ACPI: Document how to refer to LEDs from remote nodes

[Pavel Machek]

On Fri 2017-09-08 16:18:15, Sakari Ailus wrote:
> Document referring to LEDs from remote device nodes, such as from camera
> sensors.
> 
> Signed-off-by: Sakari Ailus 

> diff --git a/Documentation/acpi/dsd/leds.txt b/Documentation/acpi/dsd/leds.txt
> new file mode 100644
> index ..6217fcda15c9
> --- /dev/null
> +++ b/Documentation/acpi/dsd/leds.txt
> @@ -0,0 +1,92 @@
> +Describing and referring to LEDs in ACPI
> +
> +Individual LEDs are described by hierarchical data extension [6] nodes
> +under the device node, the LED driver chip. The "led" property in the
> +LED specific nodes tells the numerical ID of each individual LED. The
> +"led" property is used here in a similar fashion as the "reg" property
> +in DT. [3]
> +
> +Referring to LEDs in Devicetree is documented in [4], in "flash-leds"
> +property documentation. In short, LEDs are directly referred to by
> +using phandles.
> +
> +While Devicetree allows referring to any node in the tree[1], in

Documentation/devicetree/usage-model.txt talks about "device tree" or
"Device Tree" . Single word looks strange to me...

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


signature.asc
Description: Digital signature

Re: [PATCH v9 23/24] dt: bindings: smiapp: Document lens-focus and flash properties

[Pavel Machek]

On Fri 2017-09-08 16:42:26, Sakari Ailus wrote:
> Hi Pavel,
> 
> On Fri, Sep 08, 2017 at 03:36:52PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > Document optional lens-focus and flash properties for the smiapp driver.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> > > ---
> > >  Documentation/devicetree/bindings/media/i2c/nokia,smia.txt | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt 
> > > b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> > > index 855e1faf73e2..a052969365d9 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> > > +++ b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> > > @@ -27,6 +27,8 @@ Optional properties
> > >  - nokia,nvm-size: The size of the NVM, in bytes. If the size is not 
> > > given,
> > >the NVM contents will not be read.
> > >  - reset-gpios: XSHUTDOWN GPIO
> > > +- flash-leds: One or more phandles to refer to flash LEDs
> > > +- lens-focus: Phandle for lens focus
> > 
> > Should we simply reference the generic documentation here? If it needs
> > changing, it will be easier changing single place.
> 
> Good question.
> 
> Ideally the properties at least would never change; we do have a common
> parser for the properties which is part of the patchset so in theory it
> would be possible to change the documentation in a backward-compatible way.
> 
> I added these properties as well as all the other properties supported by
> the sensor driver are documented here. That has been the practice AFAIU,
> albeit omissions do happen occasionally.
> 
> The issue I see with omitting the documentation here is that the user
> otherwise won't know whether a driver uses a given property or not: it
> shouldn't be necessary to read driver code to write DT source.

I see... OTOH documentation in central place _is_ a bit more verbose:

+- flash-leds: An array of phandles, each referring to a flash LED, a sub-node
+  of the LED driver device node.

What about:

flash-leds, lens-focus: see 

?

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


signature.asc
Description: Digital signature

Re: [PATCH 1/1] media: i2c: as3645a: Remove driver

[Pavel Machek]

On Fri 2017-09-08 16:51:40, Sakari Ailus wrote:
1;2802;0c> Remove the V4L2 AS3645A sub-device driver in favour of the LED flash 
class
> driver for the same hardware, drivers/leds/leds-as3645a.c. The latter uses
> the V4L2 flash LED class framework to provide V4L2 sub-device interface.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

Thanks!
Pavel

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


signature.asc
Description: Digital signature

Re: [PATCH 3/3] as3645a: Use integer numbers for parsing LEDs

[Pavel Machek]

On Fri 2017-09-08 16:23:34, Sakari Ailus wrote:
> Hi Pavel,
> 
> Thanks for the review.
> 
> On Fri, Sep 08, 2017 at 03:17:58PM +0200, Pavel Machek wrote:
> > On Fri 2017-09-08 15:42:13, Sakari Ailus wrote:
> > > Use integer numbers for LEDs, 0 is the flash and 1 is the indicator.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> > 
> > Dunno. Old code is shorter, old device tree is shorter, ... IMO both
> > versions are fine, because the LEDs are really different. Do we have
> > documentation somewhere saying that reg= should be used for this? Are
> > you doing this for consistency?
> 
> Well, actually for ACPI support. :-) It requires less driver changes this
> way. See 17th and 18th patches in "[PATCH v9 00/23] Unified fwnode endpoint
> parser, async sub-device notifier support, N9 flash DTS".

ACPI, I hate ACPI.

> A number of chips have LED binding that is aligned, see e.g.
> Documentation/devicetree/bindings/leds/leds-bcm6328.txt .

Ok, yes, that's common way LED controllers are handled. Usually all
the LEDs are "same", but...

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v9 23/24] dt: bindings: smiapp: Document lens-focus and flash properties

[Pavel Machek]

Hi!

> Document optional lens-focus and flash properties for the smiapp driver.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  Documentation/devicetree/bindings/media/i2c/nokia,smia.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt 
> b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> index 855e1faf73e2..a052969365d9 100644
> --- a/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/nokia,smia.txt
> @@ -27,6 +27,8 @@ Optional properties
>  - nokia,nvm-size: The size of the NVM, in bytes. If the size is not given,
>the NVM contents will not be read.
>  - reset-gpios: XSHUTDOWN GPIO
> +- flash-leds: One or more phandles to refer to flash LEDs
> +- lens-focus: Phandle for lens focus

Should we simply reference the generic documentation here? If it needs
changing, it will be easier changing single place.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 3/3] as3645a: Use integer numbers for parsing LEDs

[Pavel Machek]

On Fri 2017-09-08 15:42:13, Sakari Ailus wrote:
> Use integer numbers for LEDs, 0 is the flash and 1 is the indicator.
> 
> Signed-off-by: Sakari Ailus 

Dunno. Old code is shorter, old device tree is shorter, ... IMO both
versions are fine, because the LEDs are really different. Do we have
documentation somewhere saying that reg= should be used for this? Are
you doing this for consistency?

Best regards,
Pavel

>  arch/arm/boot/dts/omap3-n950-n9.dtsi |  8 ++--
>  drivers/leds/leds-as3645a.c  | 26 --


> @@ -267,15 +267,19 @@
>   clock-frequency = <40>;
>  
>   as3645a@30 {
> + #address-cells = <1>;
> + #size-cells = <0>;
>   reg = <0x30>;
>   compatible = "ams,as3645a";
> - flash {
> + flash@0 {
> + reg = <0x0>;
>   flash-timeout-us = <15>;
>   flash-max-microamp = <32>;
>   led-max-microamp = <6>;
>   ams,input-max-microamp = <175>;
>   };
> - indicator {
> + indicator@1 {
> + reg = <0x1>;
>   led-max-microamp = <1>;
>   };
>   };
> diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
> index e3f89c6130d2..605e0c64e974 100644
> --- a/drivers/leds/leds-as3645a.c
> +++ b/drivers/leds/leds-as3645a.c
> @@ -112,6 +112,10 @@
>  #define AS_PEAK_mA_TO_REG(a) \
>   ((min_t(u32, AS_PEAK_mA_MAX, a) - 1250) / 250)
>  
> +/* LED numbers for Devicetree */
> +#define AS_LED_FLASH 0
> +#define AS_LED_INDICATOR 1
> +
>  enum as_mode {
>   AS_MODE_EXT_TORCH = 0 << AS_CONTROL_MODE_SETTING_SHIFT,
>   AS_MODE_INDICATOR = 1 << AS_CONTROL_MODE_SETTING_SHIFT,
> @@ -491,10 +495,29 @@ static int as3645a_parse_node(struct as3645a *flash,
> struct device_node *node)
>  {
>   struct as3645a_config *cfg = >cfg;
> + struct device_node *child;
>   const char *name;
>   int rval;
>  
> - flash->flash_node = of_get_child_by_name(node, "flash");
> + for_each_child_of_node(node, child) {
> + u32 id = 0;
> +
> + of_property_read_u32(child, "reg", );
> +
> + switch (id) {
> + case AS_LED_FLASH:
> + flash->flash_node = of_node_get(child);
> + break;
> + case AS_LED_INDICATOR:
> + flash->indicator_node = of_node_get(child);
> + break;
> + default:
> + dev_warn(>client->dev,
> +  "unknown LED %u encountered, ignoring\n", id);
> + break;
> + }
> + }
> +
>   if (!flash->flash_node) {
>   dev_err(>client->dev, "can't find flash node\n");
>   return -ENODEV;
> @@ -538,7 +561,6 @@ static int as3645a_parse_node(struct as3645a *flash,
>>peak);
>   cfg->peak = AS_PEAK_mA_TO_REG(cfg->peak);
>  
> - flash->indicator_node = of_get_child_by_name(node, "indicator");
>   if (!flash->indicator_node) {
>   dev_warn(>client->dev,
>"can't find indicator node\n");

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


signature.asc
Description: Digital signature

Re: [PATCH 1/3] as3645a: Use ams,input-max-microamp as documented in DT bindings

[Pavel Machek]

On Fri 2017-09-08 15:42:11, Sakari Ailus wrote:
> DT bindings document the property "ams,input-max-microamp" that limits the
> chip's maximum input current. The driver and the DTS however used
> "peak-current-limit" property. Fix this by using the property documented
> in DT binding documentation.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v8 03/21] v4l: async: Use more intuitive names for internal functions

[Pavel Machek]

On Tue 2017-09-05 16:05:35, Sakari Ailus wrote:
> Rename internal functions to make the names of the functions better
> describe what they do.
> 
>   Old nameNew name
>   v4l2_async_test_notify  v4l2_async_match_notify
>   v4l2_async_belongs  v4l2_async_find_match
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v8 01/21] v4l: fwnode: Move KernelDoc documentation to the header

[Pavel Machek]

On Tue 2017-09-05 16:05:33, Sakari Ailus wrote:
> In V4L2 the practice is to have the KernelDoc documentation in the header
> and not in .c source code files. This consequientally makes the V4L2
> fwnode function documentation part of the Media documentation build.
> 
> Also correct the link related function and argument naming in
> documentation.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
> Acked-by: Hans Verkuil <hans.verk...@cisco.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v7 11/18] v4l: async: Register sub-devices before calling bound callback

[Pavel Machek]

On Sun 2017-09-03 20:49:51, Sakari Ailus wrote:
> Register the sub-device before calling the notifier's bound callback.
> Doing this the other way around is problematic as the struct v4l2_device
> has not assigned for the sub-device yet and may be required by the bound
> callback.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>

Acked-by: Pavel Machek <pa...@ucw.cz>


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


signature.asc
Description: Digital signature

Re: [PATCH v7 1/1] dt: bindings: smiapp: Document lens-focus and flash properties

[Pavel Machek]

On Sun 2017-09-03 23:18:05, Sakari Ailus wrote:
> Document optional lens-focus and flash properties for the smiapp driver.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>

Acked-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature

Re: [PATCH v3 2/3] leds: as3645a: Add LED flash class driver

[Pavel Machek]

On Thu 2017-09-07 17:52:05, Sakari Ailus wrote:
> On Thu, Sep 07, 2017 at 02:50:27PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > On Mon, Aug 28, 2017 at 01:04:51PM +0200, Pavel Machek wrote:
> > > > On Wed 2017-08-23 11:10:59, Sakari Ailus wrote:
> > > > > Add a LED flash class driver for the as3654a flash controller. A V4L2 
> > > > > flash
> > > > > driver for it already exists (drivers/media/i2c/as3645a.c), and this 
> > > > > driver
> > > > > is based on that.
> > > > 
> > > > We do not want to have two drivers for same hardware... how is that
> > > > supposed to work?
> > > 
> > > No, we don't. The intent is to remove the other driver later on, as it's
> > > implemented as a V4L2 sub-device driver. It also lacks DT support.
> > 
> > Could we perhaps remove the driver with the same patch that merges
> > this?
> 
> This patch is already merged, but I can submit another patch removing the
> other driver.

Yes, that would be nice.

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


signature.asc
Description: Digital signature

Re: [PATCH v3 2/3] leds: as3645a: Add LED flash class driver

[Pavel Machek]

Hi!

> On Mon, Aug 28, 2017 at 01:04:51PM +0200, Pavel Machek wrote:
> > On Wed 2017-08-23 11:10:59, Sakari Ailus wrote:
> > > Add a LED flash class driver for the as3654a flash controller. A V4L2 
> > > flash
> > > driver for it already exists (drivers/media/i2c/as3645a.c), and this 
> > > driver
> > > is based on that.
> > 
> > We do not want to have two drivers for same hardware... how is that
> > supposed to work?
> 
> No, we don't. The intent is to remove the other driver later on, as it's
> implemented as a V4L2 sub-device driver. It also lacks DT support.

Could we perhaps remove the driver with the same patch that merges
this?

Having two drivers would be confusing.

> > Yes, we might want to have both LED and v4l2 interface for a single
> > LED, because v4l2 is just too hairy to use, but we should not
> > duplicate drivers for that.
> > 
> > We _might_ want to do some helpers; as these LED drivers are all quite
> > similar, it should be possible to have "flash led driver" and just
> > link it to v4l2 and LED interfaces...
> 
> This is what the new LED (flash) class driver does. Feature-wise it's a
> superset of the old one. There's a minor matter left, though, which is
> querying the flash strobe status which the old driver did and the new one
> does not do yet. I don't know if anyone ever even used that feature though.

Yes, I don't think anyone will notice...

Anyway,

Acked-by: Pavel Machek <pa...@ucw.cz>
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v7 01/18] v4l: fwnode: Move KernelDoc documentation to the header

[Pavel Machek]

On Sun 2017-09-03 20:49:41, Sakari Ailus wrote:
> In V4L2 the practice is to have the KernelDoc documentation in the header
> and not in .c source code files. This consequientally makes the V4L2

consequientally: spelling?

> fwnode function documentation part of the Media documentation build.
> 
> Also correct the link related function and argument naming in
> documentation.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>

Something funny going on with utf-8 here.

Acked-by: Pavel Machek <pa...@ucw.cz>
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a

[Pavel Machek]

Hi!

> Thanks for the review!
> 
> On 08/28/17 13:33, Pavel Machek wrote:
> > Hi!
> > 
> >> +
> >> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
> >> +and b are included in the range.
> > 
> > Normally I've seen <a, b> for closed ranges, (a, b) for open
> > ranges. Is that different in your country?
> 
> I guess there are different notations. :-) I've seen regular parentheses
> being used for open ranges, too, but not < and >.
> 
> Open range is documented in a related well written Wikipedia article:
> 
> <URL:https://en.wikipedia.org/wiki/Open_range>
> 
> Are there such open ranges in Czechia? For instance, reindeer generally
> roam freely in Finnish Lappland.

:-). Well, we have pigs and roes roaming freely in the woods, but
would not normally call it open range.

> What comes to the patch, I guess "interval" could be a more appropriate
> term to use in this case:
> 
> <URL:https://en.wikipedia.org/wiki/Interval_(mathematics)>
> 
> The patch is in line with the Wikipedia article in notation but not in
> terminology. I'll send a fix.

Ok, that was really nitpicking, thanks!
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v3 2/3] leds: as3645a: Add LED flash class driver

[Pavel Machek]

On Wed 2017-08-23 11:10:59, Sakari Ailus wrote:
> Add a LED flash class driver for the as3654a flash controller. A V4L2 flash
> driver for it already exists (drivers/media/i2c/as3645a.c), and this driver
> is based on that.

We do not want to have two drivers for same hardware... how is that
supposed to work?

Yes, we might want to have both LED and v4l2 interface for a single
LED, because v4l2 is just too hairy to use, but we should not
duplicate drivers for that.

We _might_ want to do some helpers; as these LED drivers are all quite
similar, it should be possible to have "flash led driver" and just
link it to v4l2 and LED interfaces...

Pavel


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


signature.asc
Description: Digital signature

Re: [PATCH v3 1/3] dt: bindings: Document DT bindings for Analog devices as3645a

[Pavel Machek]

Hi!

> +led-max-microamp: Maximum torch (assist) current in microamperes. The
> +   value must be in the range between [2, 16] and
> +   divisible by 2.
> +ams,input-max-microamp: Maximum flash controller input current. The

"in microamperes".

> + value must be in the range [125, 200]
> + and divisible by 5.

Is there any reason for "ams," prefix here?

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


signature.asc
Description: Digital signature

Re: [PATCH v2 1/3] dt: bindings: Document DT bindings for Analog devices as3645a

[Pavel Machek]

Hi!

> +
> +Ranges below noted as [a, b] are closed ranges between a and b, i.e. a
> +and b are included in the range.

Normally I've seen <a, b> for closed ranges, (a, b) for open
ranges. Is that different in your country?

Otherwise

Acked-by: Pavel Machek <pa...@ucw.cz>


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


signature.asc
Description: Digital signature

Re: [RFC 00/19] Async sub-notifiers and how to use them

[Pavel Machek]

Hi!

> >> Is this always the case? In the R-Car VIN driver I register the video 
> >> devices using video_register_device() in the complete handler. Am I 
> >> doing things wrong in that driver? I had a patch where I moved the 
> >> video_register_device() call to probe time but it got shoot down in 
> >> review and was dropped.
> > 
> > I don't think the current implementation is wrong, it's just different
> > from other drivers; there's really no requirement regarding this AFAIU.
> > It's one of the things where no attention has been paid I presume.
> 
> It actually is a requirement: when a device node appears applications can
> reasonably expect to have a fully functioning device. True for any device
> node. You don't want to have to wait until some unspecified time before
> the full functionality is there.

Well... /dev/sdb appears, but you still get -ENOMEDIA before user
presses "Turn on USB storage" button on android phone.

So I agree it is not desirable, but it sometimes happens.

Pavel

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


signature.asc
Description: Digital signature

Re: [PATCH 1/1] et8ek8: Decrease stack usage

[Pavel Machek]

On Wed 2017-08-16 10:33:45, Sakari Ailus wrote:
> The et8ek8 driver combines I²C register writes to a single array that it
> passes to i2c_transfer(). The maximum number of writes is 48 at once,
> decrease it to 8 and make more transfers if needed, thus avoiding a
> warning on stack usage.
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> ---
> Pavel: this is just compile tested. Could you test it on N900, please?

(More than 1 et8ek8 device makes static bad idea).

Acked-by: Pavel Machek <pa...@ucw.cz>
Tested-by: Pavel Machek <pa...@ucw.cz>

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


signature.asc
Description: Digital signature