archive mirror
 help / color / mirror / Atom feed
From: Hugues FRUCHET <>
To: Sakari Ailus <>
Cc: Alexandre TORGUE <>,
	Mauro Carvalho Chehab <>,
	Hans Verkuil <>,
	"" <>,
	"" <>,
	Benjamin Gaignard <>,
	Yannick FERTRE <>,
	Philippe CORNU <>,
	Mickael GUENE <>
Subject: Re: [PATCH v2 0/3] DCMI bridge support
Date: Mon, 24 Jun 2019 10:10:05 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20190620161721.h3wn4nibomrvriw4@kekkonen.localdomain>

Hi Sakari,

 > - Where's the sub-device representing the bridge itself?
This is pointed by [1]: drivers/media/i2c/st-mipid02.c

 > - As the driver becomes MC-centric, crop configuration takes place
 >   V4L2 sub-device interface, not through the video device node.
 > - Same goes for accessing sensor configuration: it does not take place
 >   through video node but through the sub-device nodes.

Our objective is to be able to support either a simple parallel sensor
or a CSI-2 sensor connected through a bridge without any changes on 
userspace side because no additional processing or conversion involved, 
only deserialisation is m.
With the proposed set of patches, we succeeded to do so, the same 
non-regression tests campaign is passed with OV5640 parallel sensor 
(STM32MP1 evaluation board) or OV5640 CSI-2 sensor (Avenger96 board with 
D3 mezzanine board).

We don't want driver to be MC-centric, media controller support was 
required only to get access to the set of functions needed to link and
walk trough subdevices: media_create_pad_link(), 
media_entity_remote_pad(), etc...

We did a try with the v1 version of this patchset, delegating subdevices 
handling to userspace, by using media-controller, but this require to 
configure first the pipeline for each single change of resolution and 
format before making any capture using v4l2-ctl or GStreamer, quite 
heavy in fact.
Benjamin did another try using new libcamera codebase, but even for a 
basic capture use-case, negotiation code is quite tricky in order to
match the right subdevices bus format to the required V4L2 format.
Moreover, it was not clear how to call libcamera library prior to any
v4l2-ctl or GStreamer calls.

Adding 100 lines of code into DCMI to well configure resolution and 
formats fixes the point and allows us to keep backward compatibility
as per our objective, so it seems far more reasonable to us to do so
even if DCMI controls more than the subdevice it is connected to.
Moreover we found similar code in other video interfaces code like 
qcom/camss/camss.c and xilinx/xilinx-dma.c, controlling the whole 
pipeline, so it seems to us quite natural to go this way.

To summarize, if we cannot do the negotiation within kernel, delegating
this to userspace implies far more complexity and breaks compatibility
with existing applications without adding new functionalities.

Having all that in mind, what should be reconsidered in your opinion 
Sakari ? Do you have some alternatives ?

Best regards,

On 6/20/19 6:17 PM, Sakari Ailus wrote:
> Hi Hugues,
> On Tue, Jun 11, 2019 at 10:48:29AM +0200, Hugues Fruchet wrote:
>> This patch serie allows to connect non-parallel camera sensor to
>> DCMI thanks to a bridge connected in between such as STMIPID02 [1].
>> Media controller support is introduced first, then support of
>> several sub-devices within pipeline with dynamic linking
>> between them.
>> In order to keep backward compatibility with applications
>> relying on V4L2 interface only, format set on video node
>> is propagated to all sub-devices connected to camera interface.
>> [1]
> General notes on the set, not related to any single patch:
> - Where's the sub-device representing the bridge itself?
> - As the driver becomes MC-centric, crop configuration takes place through
>    V4L2 sub-device interface, not through the video device node.
> - Same goes for accessing sensor configuration: it does not take place
>    through video node but through the sub-device nodes.

  reply	other threads:[~2019-06-24 10:10 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-11  8:48 [PATCH v2 0/3] DCMI bridge support Hugues Fruchet
2019-06-11  8:48 ` [PATCH v2 1/3] media: stm32-dcmi: improve sensor subdev naming Hugues Fruchet
2019-06-20 15:26   ` Sakari Ailus
2019-07-02 15:21     ` Hugues FRUCHET
2019-06-11  8:48 ` [PATCH v2 2/3] media: stm32-dcmi: add media controller support Hugues Fruchet
2019-06-20 12:01   ` Hans Verkuil
2019-07-02 15:18     ` Hugues FRUCHET
2019-06-20 16:13   ` Sakari Ailus
2019-07-02 15:29     ` Hugues FRUCHET
2019-06-11  8:48 ` [PATCH v2 3/3] media: stm32-dcmi: add support of several sub-devices Hugues Fruchet
2019-06-20 15:54   ` Sakari Ailus
2019-07-02 15:26     ` Hugues FRUCHET
2019-06-20 16:17 ` [PATCH v2 0/3] DCMI bridge support Sakari Ailus
2019-06-24 10:10   ` Hugues FRUCHET [this message]
2019-06-26 17:25     ` Laurent Pinchart
2019-06-27 12:38       ` Hugues FRUCHET
2019-06-27 13:38         ` Laurent Pinchart
2019-07-05  7:55           ` Sakari Ailus
2019-07-05  8:04             ` Laurent Pinchart
2019-07-05  9:16               ` Sakari Ailus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).