linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion to media controller API
@ 2011-07-08 15:25 Sylwester Nawrocki
  2011-07-14 16:27 ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: Sylwester Nawrocki @ 2011-07-08 15:25 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab

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,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion to media controller API
  2011-07-08 15:25 [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion to media controller API Sylwester Nawrocki
@ 2011-07-14 16:27 ` Mauro Carvalho Chehab
  2011-07-14 19:07   ` Sylwester Nawrocki
  0 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2011-07-14 16:27 UTC (permalink / raw)
  To: Sylwester Nawrocki; +Cc: linux-media

Em 08-07-2011 12:25, Sylwester Nawrocki escreveu:
> 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()

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

>       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.
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
optionally allow changing it via the media device, but it should not be
a mandatory requirement, as the media device API is optional.

>       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,


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion  to media controller API
  2011-07-14 16:27 ` Mauro Carvalho Chehab
@ 2011-07-14 19:07   ` Sylwester Nawrocki
  2011-07-14 21:51     ` [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion? " Sakari Ailus
  0 siblings, 1 reply; 7+ messages in thread
From: Sylwester Nawrocki @ 2011-07-14 19:07 UTC (permalink / raw)
  To: linux-media

Hi Mauro,

On Thu, 14 Jul 2011 13:27:17 -0300, Mauro Carvalho Chehab 
<mchehab@redhat.com> wrote:
> Em 08-07-2011 12:25, Sylwester Nawrocki escreveu:
> > 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()


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

They are registered in the media device probe callback, please see 
fimc-mdevice.c, fimc_md_probe(). After all the modules they depend on 
were initialized and registered. 
This is needed to assure proper initialization sequence. 
I also needed media device instance at hand when registering a video 
node.

> >       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.
> 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
> optionally allow changing it via the media device, but it should 
not be
> 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.

--
Regards,
Sylwester


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion? to media controller API
  2011-07-14 19:07   ` Sylwester Nawrocki
@ 2011-07-14 21:51     ` Sakari Ailus
  2011-07-15 18:36       ` Sylwester Nawrocki
  0 siblings, 1 reply; 7+ messages in thread
From: Sakari Ailus @ 2011-07-14 21:51 UTC (permalink / raw)
  To: Sylwester Nawrocki; +Cc: linux-media

On Thu, Jul 14, 2011 at 10:07:03PM +0300, Sylwester Nawrocki wrote:
> Hi Mauro,

Hi Sylwester and Mauro,

> On Thu, 14 Jul 2011 13:27:17 -0300, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
> >Em 08-07-2011 12:25, Sylwester Nawrocki escreveu:
...
> >>       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.
> >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
> >optionally allow changing it via the media device, but it should
> not be
> >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.

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.

Cheers,

-- 
Sakari Ailus
sakari.ailus@iki.fi

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion? to media controller API
  2011-07-14 21:51     ` [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion? " Sakari Ailus
@ 2011-07-15 18:36       ` Sylwester Nawrocki
  2011-07-18  9:22         ` Sakari Ailus
  0 siblings, 1 reply; 7+ messages in thread
From: Sylwester Nawrocki @ 2011-07-15 18:36 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, mchehab

Hi Sakari,

thanks for your comments.





Sakari Ailus <sakari.ailus@iki.fi> wrote:

T>On Thu, Jul 14, 2011 at 10:07:03PM +0300, Sylwester Nawrocki wrote:
>> Hi Mauro,
>
>Hi Sylwester and Mauro,
T>
>> On Thu, 14 Jul 2011 13:27:17 -0300, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
T>> >Em 08-07-2011 12:25, Sylwester Nawrocki escreveu:
>...
>> >>       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.
>> >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
>> >optionally allow changing it via the media device, but it should
>> not be
>> >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
T>> 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.
>> 
Tr>> 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.

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.

>
>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).

-- 
Thanks,
Sylwester

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion? to media controller API
  2011-07-15 18:36       ` Sylwester Nawrocki
@ 2011-07-18  9:22         ` Sakari Ailus
  2011-07-18 20:17           ` Sylwester Nawrocki
  0 siblings, 1 reply; 7+ messages in thread
From: Sakari Ailus @ 2011-07-18  9:22 UTC (permalink / raw)
  To: Sylwester Nawrocki; +Cc: linux-media, mchehab

On Fri, Jul 15, 2011 at 09:36:50PM +0300, Sylwester Nawrocki wrote:
> Hi Sakari,
> 
> thanks for your comments.

Hi Sylwester,

You're welcome. :-)

> Sakari Ailus <sakari.ailus@iki.fi> wrote:
> 
> T>On Thu, Jul 14, 2011 at 10:07:03PM +0300, Sylwester Nawrocki wrote:
> >> Hi Mauro,
> >
> >Hi Sylwester and Mauro,
> T>
> >> On Thu, 14 Jul 2011 13:27:17 -0300, Mauro Carvalho Chehab
> >> <mchehab@redhat.com> wrote:
> T>> >Em 08-07-2011 12:25, Sylwester Nawrocki escreveu:
> >...
> >> >>       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.
> >> >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
> >> >optionally allow changing it via the media device, but it should
> >> not be
> >> >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
> T>> 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.
> >> 
> Tr>> 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.
> 
> 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.

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?

> >
> >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).

This diagram is very informative. Thanks.

-- 
Sakari Ailus
sakari.ailus@iki.fi

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion? to media controller API
  2011-07-18  9:22         ` Sakari Ailus
@ 2011-07-18 20:17           ` Sylwester Nawrocki
  0 siblings, 0 replies; 7+ messages in thread
From: Sylwester Nawrocki @ 2011-07-18 20:17 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, mchehab



Sakari Ailus <sakari.ailus@iki.fi> wrote:
>On Fri, Jul 15, 2011 at 09:36:50PM +0300, Sylwester Nawrocki wrote:
>> Hi Sakari,
>>
>> thanks for your comments.
>
>Hi Sylwester,
>
>You're welcome. :-)
>
>> Sakari Ailus <sakari.ailus@iki.fi> wrote:
>>
>> T>On Thu, Jul 14, 2011 at 10:07:03PM +0300, Sylwester Nawrocki wrote:
>> >> Hi Mauro,
>> >
>> >Hi Sylwester and Mauro,
>> >
>> >> On Thu, 14 Jul 2011 13:27:17 -0300, Mauro Carvalho Chehab
>> >> <mchehab@redhat.com> wrote:
>> >> >Em 08-07-2011 12:25, Sylwester Nawrocki escreveu:
>> >...
>> >> >>       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.
>> >> >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
>> >> >optionally allow changing it via the media device, but it should
>> >> not be
>> >> >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 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.
>> >>
>> >> 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.
>> 
>> 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?

Yes, indeed. The sensor devices cannot be really registered with a video devnode
driver, otherwise a sensor could not be attached to 2 FIMCs simultanouesly.
Some top level entity (just like the media device is) needs to hold them. 
It's just one of the issues. There is really not enough information at the
video devnode driver level for S_INPUT to make sense. 
And I believe MC API is not only optional for devices
like OMAP3 ISP or
Samsung FIMC (and the others coming?). It allows to fully cover the H/W
capabilities and better reflects what's really going on in the device.
I wouldn't bother to rewrite the driver if there still wouldn't be an essential functionality support missing. In fact one of the main decisions when planning that was to *not* use S_INPUT at all.

-- 
Regards,
Sylwester

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2011-07-18 20:18 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-08 15:25 [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion to media controller API Sylwester Nawrocki
2011-07-14 16:27 ` Mauro Carvalho Chehab
2011-07-14 19:07   ` Sylwester Nawrocki
2011-07-14 21:51     ` [GIT PATCHES FOR 3.1] s5p-fimc and noon010pc30 drivers conversion? " Sakari Ailus
2011-07-15 18:36       ` Sylwester Nawrocki
2011-07-18  9:22         ` Sakari Ailus
2011-07-18 20:17           ` Sylwester Nawrocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).