All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luca Ceresoli <luca@lucaceresoli.net>
To: Jacopo Mondi <jacopo@jmondi.org>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
	Jacopo Mondi <jacopo+renesas@jmondi.org>,
	laurent.pinchart@ideasonboard.com,
	niklas.soderlund+renesas@ragnatech.se,
	linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH v3 26/31] adv748x: csi2: add internal routing configuration
Date: Thu, 28 Mar 2019 16:17:34 +0100	[thread overview]
Message-ID: <63fd5938-0b2a-5c81-8a0a-4bd3ca07a291@lucaceresoli.net> (raw)
In-Reply-To: <20190328150847.khb2spjt2jotisfx@uno.localdomain>

Hi Jacopo,

On 28/03/19 16:08, Jacopo Mondi wrote:
> Hi Luca,
> 
> On Fri, Mar 22, 2019 at 05:52:15PM +0100, Luca Ceresoli wrote:
>> Hi,
>>
>> thanks for the follow-up.
>>
>> On 20/03/19 18:14, Jacopo Mondi wrote:
>>>>>> This is probably the wrong patch to use an example, as this one is for
>>>>>> a multiplexed interface, where there is no need to go through an
>>>>>> s_stream() for the two CSI-2 endpoints, but as you pointed out in our
>>>>>> brief offline chat, the AFE->TX routing example for this very device
>>>>>> is a good one: if we change the analogue source that is internally
>>>>>> routed to the CSI-2 output of the adv748x, do we need to s_stream(1)
>>>>>> the now routed entity and s_stream(0) on the not not-anymore-routed
>>>>>> one?
>>>>>>
>>>>>> My gut feeling is that this is up to userspace, as it should know
>>>>>> what are the requirements of the devices in the system, but this mean
>>>>>> going through an s_stream(0)/s_stream(1) sequence on the video device,
>>>>>> and that would interrupt the streaming for sure.
>>>>>>
>>>>>> At the same time, I don't feel too much at ease with the idea of
>>>>>> s_routing calling s_stream on the entity' remote subdevices, as this
>>>>>> would skip the link format validation that media_pipeline_start()
>>>>>> performs.
>>>>>
>>>>> The link validation must be done in this case as well, it may not be
>>>>> simply skipped.
>>>>
>>>> Agreed.
>>>>
>>>> The routing VS pipeline validation point is a very important one. The
>>>> current proposed workflow is:
>>>>
>>>>  1. the pipeline is validated as a whole, having knowledge of all the
>>>>     entities
>>>
>>> let me specify this to avoid confusions:
>>>      "all the entities -with an active route in the pipeline-"
>>>
>>>>  2. streaming is started
>>>>  3. s_routing is called on an entity (not on the pipeline!)
>>>>
>>>> Now the s_routing function in the entity driver is not in a good
>>>> position to validate the candidate future pipeline as a whole.
>>>>
>>>> Naively I'd say there are two possible solutions:
>>>>
>>>>  1. the s_routing reaches the pipeline first, then the new pipeline is
>>>>     computed and verified, and if verification succeeds it is applied
>>>>  2. a partial pipeline verification mechanism is added, so the entity
>>>>     receiving a s_routing request to e.g. change the sink pad can invoke
>>>>     a verification on the part of pipeline that is about to be
>>>>     activated, and if verification succeeds it is applied
>>>>
>>>> Somehow I suspect neither is trivial...
>>>
>>> I would say it is not, but if you have such a device that does not
>>> require going through a s_stream(0)/s_stream(1) cycle and all the
>>> associated re-negotiation and validations, it seems to me nothing
>>> prevents you from handling this in the driver implementation. Maybe it
>>> won't look that great, but this seems to be quite a custom design that
>>> requires all input sources to be linked to your sink pads, their
>>> format validated all at the same time, power, stream activation and
>>> internal mux configuration controlled by s_routing. Am I wrong or
>>> nothing in this series would prevent your from doing this?
>>
>> You're right, nothing prevents me from doing a custom hack for my custom
>> design. It's what I'm doing right now. My concern is whether the
>> framework will evolve to allow modifying a running pipeline without
>> custom hacks.
>>
>>> tl;dr: I would not make this something the framework should be
>>> concerned about, as there's nothing preventing you from
>>> implementing support for such a use case. But again, without a real
>>> example we can only guess, and I might be overlooking the issue or
>>> missing some relevant detail for sure.
>>
>> I'm a bit surprised in observing that my use case looks so strange,
>> perhaps yours is so different that we don't quite understand each other.
>> So below is an example of what I have in mind. Can you explain your use
>> case too?
> 
> I'm mostly interested in this series as it allows me to add data lane
> negotiation at run time. In my case, there are no stream continuity
> constraints, but I get your point here.
>>
>>
>> Here's a use case. Consider a product that takes 3 camera inputs,
>> selects one of them and produces a continuous video stream with the
>> camera image and an OSD on top of it. The selected camera can be changed
>> at any time (e.g. upon user selection).
>>
>>                   OSD FB ---.
>>                             |
>>             .--------.      V
>> Camera 0 -->|        |   .-----.
>> Camera 1 -->|  MUX   |-->| OSD |--> DMA --> /dev/video0
>> Camera 2 -->|        |   `-----'
>>             `--------'
>>
>> A prerequisite is obviously that each piece of hardware and software
>> involved is able to cope with a sudden stream change. Perhaps not that
>> common, but no rocket science.
>>
>> It looks to me that each of these pieces can be modeled as an entity and
>> the s_routing API is a perfect fit for the mux block. Am I wrong?
>>
> 
> Personally, I would say that your muxer driver, if able to switch
> source without requiring a pipeline restart, should handle it
> internally. There are specific details that the mux should be able to
> handle, and we're still pretty vague on the details. As a start, which
> is the bus type your sources connects to the muxer with? Parallel?
> CSI-2? How is the video stream multiplexed? with CSI-2 VC? Do you need
> to control your source power during inactive period?

Not completely defined, but it could be either parallel or MIPI CSI-2,
and each input could be connected either directly from the sensor or
through a ser/des. Controlling power would be important as well.

> I'm sure there
> are more questions... I agree a partial pipeline restart might be
> something to consider, but at the same time I think this is not
> something strictly required to get this series merged, isn't it?

Right. As far as I can understand this series is already OK if you need
to change routing while the stream is not running.

-- 
Luca

  reply	other threads:[~2019-03-28 15:17 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-05 18:51 [PATCH v3 00/31] v4l: add support for multiplexed streams Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 01/31] media: entity: Use pad as a starting point for graph walk Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 02/31] media: entity: Use pads instead of entities in the media graph walk stack Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 03/31] media: entity: Walk the graph based on pads Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 04/31] v4l: mc: Start walk from a specific pad in use count calculation Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 05/31] media: entity: Add iterator helper for entity pads Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 06/31] media: entity: Move the pipeline from entity to pads Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 07/31] media: entity: Use pad as the starting point for a pipeline Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 08/31] media: entity: Add has_route entity operation Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 09/31] media: entity: Add media_has_route() function Jacopo Mondi
2019-03-14 14:45   ` Luca Ceresoli
2019-03-15  9:22     ` Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 10/31] media: entity: Use routing information during graph traversal Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 11/31] media: entity: Skip link validation for pads to which there is no route to Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 12/31] media: entity: Add an iterator helper for connected pads Jacopo Mondi
2019-03-07 10:09   ` Sakari Ailus
2019-03-07 10:27     ` Ian Arkver
2019-03-07 12:38       ` Sakari Ailus
2019-03-07 20:18       ` Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 13/31] media: entity: Add only connected pads to the pipeline Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 14/31] media: entity: Add debug information in graph walk route check Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 15/31] v4l: Add bus type to frame descriptors Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 16/31] v4l: Add CSI-2 bus configuration " Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 17/31] v4l: Add stream to frame descriptor Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 18/31] v4l: subdev: Add [GS]_ROUTING subdev ioctls and operations Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 19/31] media: Documentation: Add GS_ROUTING documentation Jacopo Mondi
2019-03-07 15:19   ` Sakari Ailus
2019-03-08 13:31     ` Jacopo Mondi
2019-03-08 14:14       ` Sakari Ailus
2019-03-14 14:44     ` Luca Ceresoli
2019-03-14 14:43   ` Luca Ceresoli
2020-02-13 13:36   ` Hans Verkuil
2019-03-05 18:51 ` [PATCH v3 20/31] v4l: subdev: Take routing information into account in link validation Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 21/31] v4l: subdev: Improve link format validation debug messages Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 22/31] v4l: mc: Add an S_ROUTING helper function for power state changes Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 23/31] adv748x: csi2: add translation from pixelcode to CSI-2 datatype Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 24/31] adv748x: csi2: only allow formats on sink pads Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 25/31] adv748x: csi2: describe the multiplexed stream Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 26/31] adv748x: csi2: add internal routing configuration Jacopo Mondi
2019-03-14 14:45   ` Luca Ceresoli
2019-03-15  9:45     ` Jacopo Mondi
2019-03-15 10:06       ` Sakari Ailus
2019-03-16 10:23         ` Luca Ceresoli
2019-03-20 17:14           ` Jacopo Mondi
2019-03-22 16:52             ` Luca Ceresoli
2019-03-28 15:08               ` Jacopo Mondi
2019-03-28 15:17                 ` Luca Ceresoli [this message]
2019-03-05 18:51 ` [PATCH v3 27/31] adv748x: afe: add routing support Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 28/31] adv748x: afe: Implement has_route() Jacopo Mondi
2019-03-07 12:49   ` Sakari Ailus
2019-03-14 14:45   ` Luca Ceresoli
2019-03-05 18:51 ` [PATCH v3 29/31] rcar-csi2: use frame description information to configure CSI-2 bus Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 30/31] rcar-csi2: expose the subdevice internal routing Jacopo Mondi
2019-03-05 18:51 ` [PATCH v3 31/31] media: rcar-csi2: Implement has_route() Jacopo Mondi
2019-03-07 12:56   ` Sakari Ailus
2019-03-07 22:28     ` Jacopo Mondi
2019-03-07  9:47 ` [PATCH v3 00/31] v4l: add support for multiplexed streams Sakari Ailus
2019-03-08 13:19   ` Jacopo Mondi
2019-03-08 14:12     ` 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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=63fd5938-0b2a-5c81-8a0a-4bd3ca07a291@lucaceresoli.net \
    --to=luca@lucaceresoli.net \
    --cc=jacopo+renesas@jmondi.org \
    --cc=jacopo@jmondi.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=sakari.ailus@linux.intel.com \
    --subject='Re: [PATCH v3 26/31] adv748x: csi2: add internal routing configuration' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.