s5p-fimc and noon010pc30 drivers conversion to media controller API

Hi Mauro,

The following changes since commit 6068c012c3741537c9f965be5b4249f989aa5efc:

[media] v4l: Document V4L2 control endianness as machine endianness (2011-07-07 19:26:11 -0300)

are available in the git repository at:
git://git.infradead.org/users/kmpark/linux-2.6-samsung s5p-fimc-next

These patches convert FIMC and the sensor driver to media controller API, i.e. a top level media device is added to be able to manage at runtime attached sensors and all video processing entities present in the SoC. An additional subdev at FIMC capture driver exposes the scaler and composing functionality of the video capture IP.
The previously existing functionality is entirely retained.

I have introduced a few changes comparing to the last version (v3) sent to the ML, as commented below.

Sylwester Nawrocki (28):
s5p-fimc: Add support for runtime PM in the mem-to-mem driver
s5p-fimc: Add media entity initialization
s5p-fimc: Remove registration of video nodes from probe()

s5p-fimc: Remove sclk_cam clock handling
s5p-fimc: Limit number of available inputs to one

s5p-fimc: Remove sensor management code from FIMC capture driver
s5p-fimc: Remove v4l2_device from video capture and m2m driver
s5p-fimc: Add the media device driver
s5p-fimc: Conversion to use struct v4l2_fh
-> removed the check of return value from v4l2_fh_init as its
signature has changed

s5p-fimc: Conversion to the control framework
s5p-fimc: Add media operations in the capture entity driver
s5p-fimc: Add PM helper function for streaming control
s5p-fimc: Correct color format enumeration
s5p-fimc: Convert to use media pipeline operations

s5p-fimc: Add subdev for the FIMC processing block
-> added setting of a default capture format in device open()

s5p-fimc: Add support for camera capture in JPEG format
s5p-fimc: Add v4l2_device notification support for single frame capture
s5p-fimc: Use consistent names for the buffer list functions
s5p-fimc: Add runtime PM support in the camera capture driver
s5p-fimc: Correct crop offset alignment on exynos4
s5p-fimc: Remove single-planar capability flags
noon010pc30: Do not ignore errors in initial controls setup
noon010pc30: Convert to the pad level ops
-> removed unused variable and pad number prerequisite check
in noon010_set_fmt

noon010pc30: Clean up the s_power callback
noon010pc30: Remove g_chip_ident operation handler
s5p-csis: Handle all available power supplies
-> renamed 'supply' to 'supplies' in s5p-csis as per Laurent's
suggestion

s5p-csis: Rework of the system suspend/resume helpers
s5p-csis: Enable v4l subdev device node

 drivers/media/video/Kconfig                 |    4 +-
 drivers/media/video/noon010pc30.c           |  173 ++--
 drivers/media/video/s5p-fimc/Makefile       |    2 +-
 drivers/media/video/s5p-fimc/fimc-capture.c | 1424 +++++++++++++++++++--------
 drivers/media/video/s5p-fimc/fimc-core.c    | 1119 +++++++++++----------
 drivers/media/video/s5p-fimc/fimc-core.h    |  222 +++--
 drivers/media/video/s5p-fimc/fimc-mdevice.c |  859 ++++++++++++++++
 drivers/media/video/s5p-fimc/fimc-mdevice.h |  118 +++
 drivers/media/video/s5p-fimc/fimc-reg.c     |   76 +-
 drivers/media/video/s5p-fimc/mipi-csis.c    |   84 +-
 drivers/media/video/s5p-fimc/regs-fimc.h    |    8 +-
 include/media/s5p_fimc.h                    |   11 +
 include/media/v4l2-chip-ident.h             |    3 -
 13 files changed, 2921 insertions(+), 1182 deletions(-)
 create mode 100644 drivers/media/video/s5p-fimc/fimc-mdevice.c
 create mode 100644 drivers/media/video/s5p-fimc/fimc-mdevice.h

Regards,

Em 08-07-2011 12:25, Sylwester Nawrocki escreveu:

Quoted 3 lines

Hi Mauro,

The following changes since commit

Quoted 2 lines

[media] v4l: Document V4L2 control endianness as machine

Quoted 3 lines

are available in the git repository at:
git://git.infradead.org/users/kmpark/linux-2.6-samsung

Quoted 2 lines

These patches convert FIMC and the sensor driver to media

Quoted 1 line

i.e. a top level media device is added to be able to manage at

Quoted 1 line

attached sensors and all video processing entities present in the

Quoted 5 lines

An additional subdev at FIMC capture driver exposes the scaler and composing functionality of the video capture IP.
The previously existing functionality is entirely retained.

I have introduced a few changes comparing to the last version

Quoted 4 lines

to the ML, as commented below.

Sylwester Nawrocki (28):
s5p-fimc: Add support for runtime PM in the mem-to-mem

Quoted 2 lines

s5p-fimc: Add media entity initialization
s5p-fimc: Remove registration of video nodes from probe()

That patch seems weird for me. If they aren't registered at probe, when they're registered?

Quoted 2 lines

s5p-fimc: Remove sclk_cam clock handling
s5p-fimc: Limit number of available inputs to one

Camera sensors at FIMC input are no longer selected with S_INPUT

They will be attached to required FIMC entity through pipeline re-configuration at the media device level.

Why? The proper way to select an input is via S_INPUT. The driver

optionally allow changing it via the media device, but it should

a mandatory requirement, as the media device API is optional.

Hi Mauro,

Quoted 1 line · Mauro Carvalho Chehab

Em 08-07-2011 12:25, Sylwester Nawrocki escreveu:

Quoted 5 lines

Quoted 2 lines

s5p-fimc: Remove sclk_cam clock handling
s5p-fimc: Limit number of available inputs to one

Camera sensors at FIMC input are no longer selected with S_INPUT

ioctl.

Quoted 5 lines

They will be attached to required FIMC entity through pipeline re-configuration at the media device level.

Why? The proper way to select an input is via S_INPUT. The driver

may also

Quoted 1 line

optionally allow changing it via the media device, but it should

not be

Quoted 1 line

a mandatory requirement, as the media device API is optional.

The problem I'm trying to solve here is sharing the sensors and mipi-csi receivers between multiple FIMC H/W instances. Previously the driver supported attaching a sensor to only one selected FIMC at compile time. You could, for instance, specify all sensors as the selected FIMC's platform data and then use S_INPUT to choose between them. The sensor could not be used together with any other FIMC. But this is desired due to different capabilities of the FIMC IP instances. And now, instead of hardcoding a sensor assigment to particular video node, the sensors are bound to the media device. The media device driver takes the list of sensors and attaches them one by one to subsequent FIMC instances when it is initializing. Each sensor has a link to each FIMC but only one of them is active by default. That said an user application can use selected camera by opening corresponding video node. Which camera is at which node can be queried with G_INPUT.

I could try to implement the previous S_INPUT behaviour, but IMHO this would lead to considerable and unnecessary driver code complication due to supporting overlapping APIs.

Quoted 1 line

Hi Mauro,

Hi Sylwester and Mauro,

Quoted 2 lines

On Thu, 14 Jul 2011 13:27:17 -0300, Mauro Carvalho Chehab <[email protected]> wrote:

...

Quoted 16 lines

Quoted 5 lines

Quoted 2 lines

s5p-fimc: Remove sclk_cam clock handling
s5p-fimc: Limit number of available inputs to one

Camera sensors at FIMC input are no longer selected with S_INPUT

ioctl.

Quoted 5 lines

They will be attached to required FIMC entity through pipeline re-configuration at the media device level.

Why? The proper way to select an input is via S_INPUT. The driver

may also

Quoted 1 line

optionally allow changing it via the media device, but it should

not be

Quoted 1 line

a mandatory requirement, as the media device API is optional.

Quoted 4 lines

The problem I'm trying to solve here is sharing the sensors and mipi-csi receivers between multiple FIMC H/W instances. Previously the driver supported attaching a sensor to only one selected FIMC at compile time. You could, for instance, specify all sensors as the

Quoted 8 lines

selected FIMC's platform data and then use S_INPUT to choose between them. The sensor could not be used together with any other FIMC. But this is desired due to different capabilities of the FIMC IP instances. And now, instead of hardcoding a sensor assigment to particular video node, the sensors are bound to the media device. The media device driver takes the list of sensors and attaches them one by one to subsequent FIMC instances when it is initializing. Each sensor has a link to each FIMC but only one of them is active

Quoted 3 lines

opening corresponding video node. Which camera is at which node can be queried with G_INPUT.

Quoted 2 lines

this would lead to considerable and unnecessary driver code complication due to supporting overlapping APIs.

This sounds quite familiar and similar to OMAP 3 ISP. There's more to configure than an S_INPUT ioctl can do. Selecting sensor using S_INPUT involves a number of other decisions as well if I'm not mistaken.

Sylwester: could you provide an MC graph of the device with possibly a few
sensors attached? AFAIR media-ctl -p and dotfile.

Hi Sakari,

thanks for your comments.

Sakari Ailus <[email protected]> wrote:

Quoted 3 lines · Sylwester Nawrocki

Quoted 1 line

Hi Mauro,

Hi Sylwester and Mauro,

T>

Quoted 2 lines

Quoted 2 lines

On Thu, 14 Jul 2011 13:27:17 -0300, Mauro Carvalho Chehab <[email protected]> wrote:

T>> >Em 08-07-2011 12:25, Sylwester Nawrocki escreveu:

Quoted 17 lines

...

Quoted 16 lines

Quoted 5 lines

Quoted 2 lines

s5p-fimc: Remove sclk_cam clock handling
s5p-fimc: Limit number of available inputs to one

Camera sensors at FIMC input are no longer selected with S_INPUT

ioctl.

Quoted 5 lines

They will be attached to required FIMC entity through pipeline re-configuration at the media device level.

Why? The proper way to select an input is via S_INPUT. The driver

may also

Quoted 1 line

optionally allow changing it via the media device, but it should

not be

Quoted 1 line

a mandatory requirement, as the media device API is optional.

Quoted 12 lines

Quoted 4 lines

The problem I'm trying to solve here is sharing the sensors and mipi-csi receivers between multiple FIMC H/W instances. Previously the driver supported attaching a sensor to only one selected FIMC at compile time. You could, for instance, specify all sensors as the

Quoted 8 lines

selected FIMC's platform data and then use S_INPUT to choose between them. The sensor could not be used together with any other FIMC. But this is desired due to different capabilities of the FIMC IP instances. And now, instead of hardcoding a sensor assigment to particular video node, the sensors are bound to the media device. The media device driver takes the list of sensors and attaches them one by one to subsequent FIMC instances when it is initializing. Each sensor has a link to each FIMC but only one of them is active

T>> by default. That said an user application can use selected camera by

Quoted 3 lines

Quoted 3 lines

opening corresponding video node. Which camera is at which node can be queried with G_INPUT.

Tr>> I could try to implement the previous S_INPUT behaviour, but IMHO

Quoted 6 lines

Quoted 2 lines

this would lead to considerable and unnecessary driver code complication due to supporting overlapping APIs.

This sounds quite familiar and similar to OMAP 3 ISP. There's more to configure than an S_INPUT ioctl can do. Selecting sensor using S_INPUT involves a number of other decisions as well if I'm not mistaken.

Yes, in particular the number of subdevs in the data path may need to be changed which really belongs to the media device driver. And sensors arbitration between FIMC instances is best done at the media device layer. Same applies to the 2 external clocks for the sensors which are shared between all FIMC IP instances and the sensors. Thus switching between sensors with S_INPUT would possibly involve pipeline reconfiguration which doesn't sound like a good design.

Quoted 4 lines

Sylwester: could you provide an MC graph of the device with possibly a few
sensors attached? AFAIR media-ctl -p and dotfile.

I would be happy to provide this information but I'm in the middle of vacations and cannot really do that at the moment;) I recall pasting some links to the graph images to v4l irc channel. Here is the working one that most closely depicts current state:
http://wstaw.org/m/2011/05/26/fimc_graph__.png (yellow FIMC.n subdevs should not be there and s5p-mipi-csis.1 subdev should have links to all green FIMC.n subdevs).

Quoted 3 lines · Sylwester Nawrocki

Hi Sakari,

thanks for your comments.

Hi Sylwester,

You're welcome. :-)

Quoted 30 lines

Sakari Ailus <[email protected]> wrote:

Quoted 27 lines · Sylwester Nawrocki

Quoted 1 line

Hi Mauro,

Hi Sylwester and Mauro,

Quoted 3 lines

Quoted 1 line · Mauro Carvalho Chehab

Em 08-07-2011 12:25, Sylwester Nawrocki escreveu:

...

Quoted 19 lines

Quoted 5 lines

Quoted 2 lines

s5p-fimc: Remove sclk_cam clock handling
s5p-fimc: Limit number of available inputs to one

Camera sensors at FIMC input are no longer selected with S_INPUT

ioctl.

Quoted 5 lines

They will be attached to required FIMC entity through pipeline re-configuration at the media device level.

Why? The proper way to select an input is via S_INPUT. The driver

may also

Quoted 1 line

optionally allow changing it via the media device, but it should

not be

Quoted 1 line

a mandatory requirement, as the media device API is optional.

The problem I'm trying to solve here is sharing the sensors and mipi-csi receivers between multiple FIMC H/W instances. Previously the driver supported attaching a sensor to only one selected FIMC

at

Quoted 21 lines

Quoted 13 lines

Quoted 13 lines

compile time. You could, for instance, specify all sensors as the selected FIMC's platform data and then use S_INPUT top choose between them. The sensor could not be used together with any other FIMC. But this is desired due to different capabilities of the FIMC IP instances. And now, instead of hardcoding a sensor assigment to particular video node, the sensors are bound to the media device. The media device driver takes the list of sensors and attaches them one by one to subsequent FIMC instances when it is initializing. Each sensor has a link to each FIMC but only one of Wethem is active by default. That said an user application can use selected camera by opening corresponding video node. Which camera is at which node can be queried with G_INPUT.

Quoted 7 lines

Quoted 3 lines

I could try to implement the previous S_INPUT behaviour, but IMHO this would lead to considerable and unnecessary driver code complication due to supporting overlapping APIs.

This sounds quite familiar and similar to OMAP 3 ISP. There's more to configure than an S_INPUT ioctl can do. Selecting sensor using S_INPUT involves a number of other decisions as well if I'm not mistaken.

Quoted 7 lines

Yes, in particular the number of subdevs in the data path may need to be changed which really belongs to the media device driver. And sensors arbitration between FIMC instances is best done at the media device layer. Same applies to the 2 external clocks for the sensors Rrwhich are shared between all FIMC IP instances and the sensors. Thus switching between sensors with S_INPUT would possibly involve pipeline reconfiguration which doesn't sound like a good design.

AFAIR this was one of the major reasons the OMAP 3 ISP driver does not support S_INPUT.

I guess the real question might not even be whether to support S_INPUT or not, but how to provide a general purpose application an easy way to use the device; right?