Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1512982 > unrolled thread

Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Sakari Ailus <sakari.ailus@iki.fi> - 2016-11-01 00:00 +0100
    Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-11-01 07:40 +0100
      Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Sakari Ailus <sakari.ailus@iki.fi> - 2016-11-01 21:20 +0100
        Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-11-01 23:20 +0100
      Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Pavel Machek <pavel@ucw.cz> - 2016-11-02 09:20 +0100
        Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-11-02 09:20 +0100
    Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Pavel Machek <pavel@ucw.cz> - 2016-11-01 16:40 +0100
      Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Sakari Ailus <sakari.ailus@iki.fi> - 2016-11-01 21:10 +0100
        Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Pavel Machek <pavel@ucw.cz> - 2016-11-03 09:20 +0100
    Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Sebastian Reichel <sre@kernel.org> - 2016-11-03 23:50 +0100
      Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Sakari Ailus <sakari.ailus@iki.fi> - 2016-11-04 00:10 +0100
        Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-11-04 00:50 +0100
        Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Sebastian Reichel <sre@kernel.org> - 2016-11-04 01:10 +0100

#1512982 — Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor

Hi Pavel,

On Sun, Oct 23, 2016 at 10:33:15PM +0200, Pavel Machek wrote:
> Hi!
> 
> > 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.

> > Do all the modes work for you currently btw.?
> 
> I don't think I got 5MP mode to work. Even 2.5MP mode is tricky (needs
> a lot of continuous memory).

The OMAP 3 ISP has got an MMU, getting some contiguous memory is not really
a problem when you have a 4 GiB empty space to use.

> Anyway, I have to start somewhere, and I believe this is a good
> starting place; I'd like to get the code cleaned up and merged, then
> move to the next parts.

I wonder if something else could be the problem. I think the data rate is
higher in the 5 MP mode, and that might be the reason. I don't remember how
similar is the clock tree in the 3430 to the 3630. Could it be that the ISP
clock is lower than it should be for some reason, for instance?

-- 
Kind regards,

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

[toc] | [next] | [standalone]

#1513109

Hi,

On  1.11.2016 00:54, Sakari Ailus wrote:
> Hi Pavel,
>
> On Sun, Oct 23, 2016 at 10:33:15PM +0200, Pavel Machek wrote:
>> Hi!
>>
>>> 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.
>

Yes, lets merge what we have till now, it will be way easier to improve 
on it once it is part of the mainline.

BTW, I have (had) patched VBS working almost without problems, when it 
comes to it I'll dig it.

>>> Do all the modes work for you currently btw.?
>>
>> I don't think I got 5MP mode to work. Even 2.5MP mode is tricky (needs
>> a lot of continuous memory).
>
> The OMAP 3 ISP has got an MMU, getting some contiguous memory is not really
> a problem when you have a 4 GiB empty space to use.
>
>> Anyway, I have to start somewhere, and I believe this is a good
>> starting place; I'd like to get the code cleaned up and merged, then
>> move to the next parts.
>
> I wonder if something else could be the problem. I think the data rate is
> higher in the 5 MP mode, and that might be the reason. I don't remember how
> similar is the clock tree in the 3430 to the 3630. Could it be that the ISP
> clock is lower than it should be for some reason, for instance?
>

IIRC I checked what Nokia kernel does, and according to my vague 
memories the frequency was the same. Still, it seems the problem is in 
ISP, it has some very fragile calculations. Yet again, having main 
camera merged will ease the problem hunting.

Regards,
Ivo

[toc] | [prev] | [next] | [standalone]

#1513491

Hi Ivaylo,

On Tue, Nov 01, 2016 at 08:36:57AM +0200, Ivaylo Dimitrov wrote:
> Hi,
> 
> On  1.11.2016 00:54, Sakari Ailus wrote:
> >Hi Pavel,
> >
> >On Sun, Oct 23, 2016 at 10:33:15PM +0200, Pavel Machek wrote:
> >>Hi!
> >>
> >>>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.
> >
> 
> Yes, lets merge what we have till now, it will be way easier to improve on
> it once it is part of the mainline.
> 
> BTW, I have (had) patched VBS working almost without problems, when it comes
> to it I'll dig it.

I wonder if I'm the only one who wonders what VBS is here. Don't tell me its
the old MS thing. :-) ;-)

-- 
Kind regards,

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

[toc] | [prev] | [next] | [standalone]

#1513536

On  1.11.2016 22:11, Sakari Ailus wrote:
> Hi Ivaylo,
>
> On Tue, Nov 01, 2016 at 08:36:57AM +0200, Ivaylo Dimitrov wrote:
>> Hi,
>>
>> On  1.11.2016 00:54, Sakari Ailus wrote:
>>> Hi Pavel,
>>>
>>> On Sun, Oct 23, 2016 at 10:33:15PM +0200, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>>> 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.
>>>
>>
>> Yes, lets merge what we have till now, it will be way easier to improve on
>> it once it is part of the mainline.
>>
>> BTW, I have (had) patched VBS working almost without problems, when it comes
>> to it I'll dig it.
>
> I wonder if I'm the only one who wonders what VBS is here. Don't tell me its
> the old MS thing. :-) ;-)
>

Oh, sorry, I thought that V(ideo) B(us) S(witch) is clear in the context :)

[toc] | [prev] | [next] | [standalone]

#1513729

[Multipart message — attachments visible in raw view] — view raw

Hi!

> >>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.
> >
> 
> Yes, lets merge what we have till now, it will be way easier to improve on
> it once it is part of the mainline.
> 
> BTW, I have (had) patched VBS working almost without problems, when it comes
> to it I'll dig it.

Do you have a version that switches on runtime?

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1513730

On  2.11.2016 10:15, Pavel Machek wrote:
> Hi!
>
>>>> 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.
>>>
>>
>> Yes, lets merge what we have till now, it will be way easier to improve on
>> it once it is part of the mainline.
>>
>> BTW, I have (had) patched VBS working almost without problems, when it comes
>> to it I'll dig it.
>
> Do you have a version that switches on runtime?
>
> Best regards,
> 									Pavel
>

IIRC yes, but I might be wrong, it was a while I was playing with it.

Ivo

[toc] | [prev] | [next] | [standalone]

#1513310

[Multipart message — attachments visible in raw view] — view raw

Hi!

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

Well, I am a little bit worried that /dev/video* entries will
renumber themself when the the front camera support is merged,
breaking userspace.

But the first step is still the same: get et8ek8 support merged :-).

> > > Do all the modes work for you currently btw.?
> > 
> > I don't think I got 5MP mode to work. Even 2.5MP mode is tricky (needs
> > a lot of continuous memory).
> 
> The OMAP 3 ISP has got an MMU, getting some contiguous memory is not really
> a problem when you have a 4 GiB empty space to use.

Ok, maybe it is something else. 2.5MP mode seems to work better when
there is free memory.

> > Anyway, I have to start somewhere, and I believe this is a good
> > starting place; I'd like to get the code cleaned up and merged, then
> > move to the next parts.
> 
> I wonder if something else could be the problem. I think the data rate is
> higher in the 5 MP mode, and that might be the reason. I don't remember how
> similar is the clock tree in the 3430 to the 3630. Could it be that the ISP
> clock is lower than it should be for some reason, for instance?

No idea, really. I'd like to get the support merged, and then debug
the code when we have common code base in the mainline.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1513472

Hi Pavel,

On Tue, Nov 01, 2016 at 04:39:21PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > 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.
> 
> Well, I am a little bit worried that /dev/video* entries will
> renumber themself when the the front camera support is merged,
> breaking userspace.
> 
> But the first step is still the same: get et8ek8 support merged :-).

Do you happen to have a patch for the DT part as well? People could more
easily test this...

> > > > Do all the modes work for you currently btw.?
> > > 
> > > I don't think I got 5MP mode to work. Even 2.5MP mode is tricky (needs
> > > a lot of continuous memory).
> > 
> > The OMAP 3 ISP has got an MMU, getting some contiguous memory is not really
> > a problem when you have a 4 GiB empty space to use.
> 
> Ok, maybe it is something else. 2.5MP mode seems to work better when
> there is free memory.

That's very odd. Do you use MMAP or USERPTR buffers btw.? I remember the
cache was different on 3430, that could be an issue as well (VIVT AFAIR, so
flushing requires making sure there are no other mappings or flushing the
entire cache).

> > > Anyway, I have to start somewhere, and I believe this is a good
> > > starting place; I'd like to get the code cleaned up and merged, then
> > > move to the next parts.
> > 
> > I wonder if something else could be the problem. I think the data rate is
> > higher in the 5 MP mode, and that might be the reason. I don't remember how
> > similar is the clock tree in the 3430 to the 3630. Could it be that the ISP
> > clock is lower than it should be for some reason, for instance?
> 
> No idea, really. I'd like to get the support merged, and then debug
> the code when we have common code base in the mainline.

Yes. It's much easier then. Which is why it'd be very nice to have the DT
content, too.

-- 
Kind regards,

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

[toc] | [prev] | [next] | [standalone]

#1514367

[Multipart message — attachments visible in raw view] — view raw

Hi!

> > > > 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.
> > 
> > Well, I am a little bit worried that /dev/video* entries will
> > renumber themself when the the front camera support is merged,
> > breaking userspace.
> > 
> > But the first step is still the same: get et8ek8 support merged :-).
> 
> Do you happen to have a patch for the DT part as well? People could more
> easily test this...

If you want complete/working tree for testing, it is at

https://git.kernel.org/cgit/linux/kernel/git/pavel/linux-n900.git/?h=camera-v4.9

If you want userspace to go with that, there's fcam-dev. It is on
gitlab:

https://gitlab.com/pavelm/fcam-dev


> > > > > Do all the modes work for you currently btw.?
> > > > 
> > > > I don't think I got 5MP mode to work. Even 2.5MP mode is tricky (needs
> > > > a lot of continuous memory).
> > > 
> > > The OMAP 3 ISP has got an MMU, getting some contiguous memory is not really
> > > a problem when you have a 4 GiB empty space to use.
> > 
> > Ok, maybe it is something else. 2.5MP mode seems to work better when
> > there is free memory.
> 
> That's very odd. Do you use MMAP or USERPTR buffers btw.? I remember the
> cache was different on 3430, that could be an issue as well (VIVT AFAIR, so
> flushing requires making sure there are no other mappings or flushing the
> entire cache).

The userland code I'm using does

 struct v4l2_requestbuffers req;
 memset(&req, 0, sizeof(req));
 req.type   = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 req.memory = V4L2_MEMORY_MMAP;
 req.count  = 8;
 printf("Reqbufs\n");
 if (ioctl(fd, VIDIOC_REQBUFS, &req) < 0) {
 ...
		

so I guess answer to your question is "MMAP". The v4l interface is at

https://gitlab.com/pavelm/fcam-dev/blob/master/src/N900/V4L2Sensor.cpp

.
Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1514905

[Multipart message — attachments visible in raw view] — view raw

Hi,

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.

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

-- Sebastian

[toc] | [prev] | [next] | [standalone]

#1514918

Hi Sebastian,

On Thu, Nov 03, 2016 at 11:48:43PM +0100, Sebastian Reichel wrote:
> Hi,
> 
> 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>

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.

> 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

-- 
Kind regards,

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

[toc] | [prev] | [next] | [standalone]

#1514937

Hi,

On  4.11.2016 01:05, Sakari Ailus wrote:
> Hi Sebastian,
>
> On Thu, Nov 03, 2016 at 11:48:43PM +0100, Sebastian Reichel wrote:
>> Hi,
>>
>> 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>
>
> 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.
>
>> 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
>

No, the reason is that hey want to keep Neo900 as close as possible to 
N900, HW wise

[toc] | [prev] | [next] | [standalone]

#1514946

[Multipart message — attachments visible in raw view] — view raw

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.

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

> > 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 :(

-- Sebastian

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web