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 \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.