archive mirror
 help / color / mirror / Atom feed
From: Tomasz Figa <>
To: Hans Verkuil <>
Cc: Mauro Carvalho Chehab <>,,
	Linux Kernel Mailing List <>,
	"Qiu, Tian Shu" <>,
	Sakari Ailus <>,
	Mauro Carvalho Chehab <>,
	"Zheng, Jian Xu" <>,
	Yong Zhi <>, Cao Bing Bu <>,
	Linux Media Mailing List <>
Subject: Re: [PATCH 0/2] media: intel-ipu3: allow the media graph to be used even if a subdev fails
Date: Wed, 14 Nov 2018 17:28:00 +0900	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hi Hans,

On Thu, Sep 27, 2018 at 7:22 PM Hans Verkuil <> wrote:
> On 09/27/2018 12:13 PM, Mauro Carvalho Chehab wrote:
> > Em Thu, 27 Sep 2018 11:52:35 +0200
> > Hans Verkuil <> escreveu:
> >
> >> Hi Javier,
> >>
> >> On 09/04/2018 01:30 PM, Javier Martinez Canillas wrote:
> >>> Hello,
> >>>
> >>> This series allows the ipu3-cio2 driver to properly expose a subset of the
> >>> media graph even if some drivers for the pending subdevices fail to probe.
> >>>
> >>> Currently the driver exposes a non-functional graph since the pad links are
> >>> created and the subdev dev nodes are registered in the v4l2 async .complete
> >>> callback. Instead, these operations should be done in the .bound callback.
> >>>
> >>> Patch #1 just adds a v4l2_device_register_subdev_node() function to allow
> >>> registering a single device node for a subdev of a v4l2 device.
> >>>
> >>> Patch #2 moves the logic of the ipu3-cio2 .complete callback to the .bound
> >>> callback. The .complete callback is just removed since is empy after that.
> >>
> >> Sorry, I missed this series until you pointed to it on irc just now :-)
> >>
> >> I have discussed this topic before with Sakari and Laurent. My main problem
> >> with this is how an application can discover that not everything is online?
> >> And which parts are offline?
> >
> > Via the media controller? It should be possible for an application to see
> > if a videonode is missing using it.
> >
> >> Perhaps a car with 10 cameras can function with 9, but not with 8. How would
> >> userspace know?
> >
> > I guess this is not the only case where someone submitted a patch for
> > a driver that would keep working if some device node registration fails.
> >
> > It could be just déjà vu, but I have a vague sensation that I merged something
> > similar to it in the past on another driver, but I can't remember any details.
> >
> >>
> >> I completely agree that we need to support these advanced scenarios (including
> >> what happens when a camera suddenly fails), but it is the userspace aspects
> >> for which I would like to see an RFC first before you can do these things.
> >
> > Dynamic runtime fails should likely rise some signal. Perhaps a sort of
> > media controller event?
> See this old discussion:
> My point is that someone needs to think about this and make a proposal.
> There may well be a simple approach, but it needs to be specced first.

In that thread, you seem to have mentioned that having a Kconfig
option, disabled by default, to allow registering an incomplete media
topology would be an acceptable option. Do you think we could revive
that idea?

Quoting some of the discussion points you mentioned:

Some discussion points:

> 1) What about adding time-out support? Today we wait forever until all components
>    are found, but wouldn't it make sense to optionally time-out? And if so, then
>    we can keep most of the code the same and decide in the complete() callback
>   whether or not we accept missing components. And decide how badly 'impaired'
>   the system is. We can also still bring up all the devices in the complete rather
>   than one-by-one as you proposed (and which I am not sure I like).

It sounds like an interesting extension, not a must have for handling
incomplete topologies.

I can also imagine the timeout handling introducing a lot of confusion
to the userspace, for example, with a long timeout, the whole
initialization would have to wait for the timeout to elapse, which for
a smartphone user could mean that they can't start the camera
application (or it times out on its own and fails) until then.

> 2) This can be hard to test, so perhaps we should support some form of error
>   injection to easily test what happens if something doesn't come up.

This is a very good point, but I think we should be able to do away
with something simpler, just blacklist particular device from being
bound to a driver. How to do it, is another question, though... (I can
imagine binding a dummy driver to the device as an example solution.)

Best regards,

      reply	other threads:[~2018-11-14  8:28 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-04 11:30 [PATCH 0/2] media: intel-ipu3: allow the media graph to be used even if a subdev fails Javier Martinez Canillas
2018-09-04 11:30 ` [PATCH 1/2] [media] v4l: allow to register dev nodes for individual v4l2 subdevs Javier Martinez Canillas
2018-09-17 16:46   ` Sakari Ailus
2018-09-17 17:13     ` Javier Martinez Canillas
2018-09-04 11:30 ` [PATCH 2/2] media: intel-ipu3: create pad links and register subdev nodes at bound time Javier Martinez Canillas
2018-09-17 16:21 ` [PATCH 0/2] media: intel-ipu3: allow the media graph to be used even if a subdev fails Javier Martinez Canillas
2018-09-27  9:52 ` Hans Verkuil
2018-09-27 10:13   ` Mauro Carvalho Chehab
2018-09-27 10:22     ` Hans Verkuil
2018-11-14  8:28       ` Tomasz Figa [this message]

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