All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@iki.fi>
To: Sebastian Reichel <sre@kernel.org>
Cc: Pavel Machek <pavel@ucw.cz>,
	ivo.g.dimitrov.75@gmail.com, pali.rohar@gmail.com,
	linux-media@vger.kernel.org, galak@codeaurora.org,
	mchehab@osg.samsung.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor
Date: Mon, 14 Nov 2016 23:58:28 +0200	[thread overview]
Message-ID: <20161114215827.GU3217@valkosipuli.retiisi.org.uk> (raw)
In-Reply-To: <20161104000525.jzouapxxwwiwdwjy@earth>

Hi Sebastian,

On Fri, Nov 04, 2016 at 01:05:25AM +0100, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Nov 04, 2016 at 01:05:01AM +0200, Sakari Ailus wrote:
> > On Thu, Nov 03, 2016 at 11:48:43PM +0100, Sebastian Reichel wrote:
> > > On Tue, Nov 01, 2016 at 12:54:08AM +0200, Sakari Ailus wrote:
> > > > > > Thanks, this answered half of my questions already. ;-)
> > > > > :-).
> > > > > 
> > > > > I'll have to go through the patches, et8ek8 driver is probably not
> > > > > enough to get useful video. platform/video-bus-switch.c is needed for
> > > > > camera switching, then some omap3isp patches to bind flash and
> > > > > autofocus into the subdevice.
> > > > > 
> > > > > Then, device tree support on n900 can be added.
> > > > 
> > > > I briefly discussed with with Sebastian.
> > > > 
> > > > Do you think the elusive support for the secondary camera is worth keeping
> > > > out the main camera from the DT in mainline? As long as there's a reasonable
> > > > way to get it working, I'd just merge that. If someone ever gets the
> > > > secondary camera working properly and nicely with the video bus switch,
> > > > that's cool, we'll somehow deal with the problem then. But frankly I don't
> > > > think it's very useful even if we get there: the quality is really bad.
> > > 
> > > If we want to keep open the option to add proper support for the
> > > second camera, we could also add the bus switch and not add the
> > > front camera node in DT. Then adding the front camera does not
> > > require DT or userspace API changes. It would need an additional
> > > DT quirk in arch/arm/mach-omap2/board-generic.c for RX51, which
> > > adds the CCP2 bus settings from the camera node to the bus
> > > switch node to keep isp_of_parse_node happy. That should be
> > > easy to implement and not add much delay in upstreaming.
> > 
> > By adding the video bus switch we have a little bit more complex system as a
> > whole. The V4L2 async does not currently support this. There's more here:
> > 
> > <URL:http://www.spinics.net/lists/linux-media/msg107262.html>
> 
> I'm not sure what part relevant for video-bus-switch is currently
> not supported?
> 
> video-bus-switch registers its own async notifier and only registers
> itself as subdevices to omap3isp, once its own subdevices have been
> registered successfully.

Do you happen to have patches for this?

I still think we should clean up the V4L2 async framework though.

> 
> > What I thought was that once we have everything that's required in
> > place, we can just change what's in DT. But the software needs to
> > continue to work with the old DT content.
> 
> Right, so DT is not a problem. But adding the switch would change
> the media-graph, which is exposed to userspace.

Well, yes, indeed. We'll have those cases coming anyway, as support for
multiple streams over a single link is added. In some cases more sub-devices
will be needed to expose all the necessary configurability to the user.

> 
> > > For actually getting both cameras available with runtime-switching
> > > the proper solution would probably involve moving the parsing of
> > > the bus-settings to the sensor driver and providing a callback.
> > > This callback can be called by omap3isp when it wants to configure
> > > the phy (which is basically when it starts streaming). That seems
> > > to be the only place needing the buscfg anyways.
> > > 
> > > Then the video-bus-switch could do something like this (pseudocode):
> > > 
> > > static void get_buscfg(struct *this, struct *buscfg) {
> > >     if (selected_cam == 0)
> > >         return this->sensor_a->get_buscfg(buscfg);
> > >     else
> > >         return this->sensor_b->get_buscfg(buscfg);
> > > }
> > > 
> > > Regarding the usefulness: I noticed, that the Neo900 people also
> > > plan to have the bus-switch [0]. It's still the same crappy front-cam,
> > > though. Nevertheless it might be useful for testing. It has nice
> > > test-image capabilities, which might be useful for regression
> > > testing once everything is in place.
> > > 
> > > [0] http://neo900.org/stuff/block-diagrams/neo900/neo900.html
> > 
> > Seriously? I suppose there should be no need for that anymore, is there?
> > 
> > I think they wanted to save one GPIO in order to shave off 0,0001 cents from
> > the manufacturing costs or something like that. And the result is...
> > painful. :-I
> 
> CSI1/CCP2 is more than a single I/O pin, isn't it? Or do you
> reference to the GPIO dual use to enable frontcam and switch
> between the cameras? That is indeed a really ugly solution :(

The GPIO, yes. It was a really bad idea...

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

  reply	other threads:[~2016-11-14 21:59 UTC|newest]

Thread overview: 111+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-23 20:03 [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Pavel Machek
2016-10-23 20:19 ` Sakari Ailus
2016-10-23 20:33   ` Pavel Machek
2016-10-31 22:54     ` Sakari Ailus
2016-11-01  6:36       ` Ivaylo Dimitrov
2016-11-01 20:11         ` Sakari Ailus
2016-11-01 22:14           ` Ivaylo Dimitrov
2016-11-02  8:15         ` Pavel Machek
2016-11-02  8:16           ` Ivaylo Dimitrov
2016-11-01 15:39       ` Pavel Machek
2016-11-01 20:08         ` Sakari Ailus
2016-11-03  8:14           ` Pavel Machek
2016-11-03 22:48       ` Sebastian Reichel
2016-11-03 23:05         ` Sakari Ailus
2016-11-03 23:40           ` Ivaylo Dimitrov
2016-11-04  0:05           ` Sebastian Reichel
2016-11-14 21:58             ` Sakari Ailus [this message]
2016-11-15  0:53               ` Sebastian Reichel
2016-11-15 10:54         ` Pavel Machek
2016-11-15 22:55           ` Sakari Ailus
2016-10-23 20:40   ` Pavel Machek
2016-10-31 22:58     ` Sakari Ailus
2016-11-02  0:45       ` Laurent Pinchart
2016-10-23 20:47   ` Pavel Machek
2016-12-13 21:05   ` Pavel Machek
2016-12-18 21:56     ` Sakari Ailus
2016-11-19 23:29 ` Sakari Ailus
2016-11-20 10:02   ` Pavel Machek
2016-11-20 15:20   ` Pavel Machek
2016-11-20 15:21   ` Pavel Machek
2016-11-20 15:31   ` Pavel Machek
2016-12-14 12:24   ` [PATCH v5] " Pavel Machek
2016-12-14 13:03     ` Pali Rohár
2016-12-14 15:52       ` Ivaylo Dimitrov
2016-12-14 20:12       ` Pavel Machek
2016-12-14 22:07         ` Pali Rohár
2016-12-14 22:35           ` Pavel Machek
2016-12-18 22:01         ` Sakari Ailus
2016-12-20 12:37           ` Pavel Machek
2016-12-20 14:01             ` Sakari Ailus
2016-12-20 22:42               ` Pavel Machek
2016-12-21 13:42     ` Sakari Ailus
2016-12-21 22:42       ` Pavel Machek
2016-12-21 23:29         ` Sakari Ailus
2016-12-22  9:34           ` Pavel Machek
2016-12-22 10:01     ` [PATCH v6] " Pavel Machek
2016-12-22 13:39       ` [RFC/PATCH] media: Add video bus switch Pavel Machek
2016-12-22 14:32         ` Sebastian Reichel
2016-12-22 20:53           ` Pavel Machek
2016-12-22 23:11             ` Sebastian Reichel
2016-12-22 22:42           ` Pavel Machek
2016-12-22 23:40             ` Sebastian Reichel
2016-12-23 11:42               ` Pavel Machek
2016-12-23 18:53                 ` Ivaylo Dimitrov
2016-12-23 20:56               ` Pavel Machek
2016-12-24 14:26               ` Pavel Machek
2016-12-24 14:43                 ` Pavel Machek
2016-12-24 15:20         ` [PATCH] " Pavel Machek
2016-12-24 18:35           ` kbuild test robot
2017-01-12 11:17           ` Pavel Machek
2017-02-03 22:25             ` Sakari Ailus
2017-02-05 22:16               ` Pavel Machek
2017-02-05 22:44                 ` Sakari Ailus
2017-02-03 12:35           ` [PATCH] devicetree: " Pavel Machek
2017-02-03 12:35             ` Pavel Machek
2017-02-03 13:07             ` Sakari Ailus
2017-02-03 21:06               ` Pavel Machek
2017-02-03 21:34                 ` Sakari Ailus
2017-02-04 21:56                   ` Pavel Machek
2017-02-04 22:33                     ` Sakari Ailus
2017-02-04 22:33                       ` Sakari Ailus
2017-02-05 21:12                       ` Pavel Machek
2017-02-05 23:40                         ` Sebastian Reichel
2017-02-05 23:40                           ` Sebastian Reichel
2017-02-06  9:37                           ` [PATCH] media: add operation to get configuration of "the other side" of the link Pavel Machek
2017-12-19 15:43                             ` Sakari Ailus
2017-12-20 17:54                     ` [PATCH] devicetree: Add video bus switch Laurent Pinchart
2017-12-20 17:54                       ` Laurent Pinchart
2017-12-21  9:05                       ` Sakari Ailus
2017-12-21  9:05                         ` Sakari Ailus
2017-12-21 16:36                       ` Ivaylo Dimitrov
2017-12-21 16:36                         ` Ivaylo Dimitrov
2017-12-22  9:24                       ` Pavel Machek
2017-12-22  9:24                         ` Pavel Machek
2017-02-03 13:32             ` Pali Rohár
2017-02-03 21:07               ` Pavel Machek
2017-02-03 21:07                 ` Pavel Machek
2017-02-04  1:04                 ` Sebastian Reichel
2017-02-04  1:04                   ` Sebastian Reichel
2017-02-08 21:36             ` Rob Herring
2017-02-08 22:30               ` Pavel Machek
2017-02-09 23:02                 ` Rob Herring
2017-02-09 23:03                   ` Rob Herring
2017-02-10 19:54                     ` Pavel Machek
2017-02-10 19:54                       ` Pavel Machek
2017-02-10 22:17                       ` Sakari Ailus
2017-02-10 22:17                         ` Sakari Ailus
2017-02-13  9:54                         ` Pavel Machek
2017-02-13  9:54                           ` Pavel Machek
2017-02-13 10:20                           ` Sakari Ailus
2017-02-13 10:20                             ` Sakari Ailus
2017-03-02  8:54                             ` Pavel Machek
2017-02-08 22:34               ` Pavel Machek
2017-02-09 22:58                 ` Rob Herring
2017-02-10 21:17                   ` Pavel Machek
2017-02-10 21:17                     ` Pavel Machek
2016-12-27  9:26       ` [PATCH v6] media: Driver for Toshiba et8ek8 5MP sensor Sakari Ailus
2016-12-27 20:45         ` Pavel Machek
2016-12-27 20:59           ` [PATCH] mark myself as mainainer for camera on N900 Pavel Machek
2016-12-27 23:57             ` Sebastian Reichel
2017-01-25 13:48               ` 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=20161114215827.GU3217@valkosipuli.retiisi.org.uk \
    --to=sakari.ailus@iki.fi \
    --cc=galak@codeaurora.org \
    --cc=ivo.g.dimitrov.75@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@osg.samsung.com \
    --cc=pali.rohar@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=sre@kernel.org \
    /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.