omap3-isp status

omap3-isp status

[Enrico]

Hi all,

since we are all interested in this driver (and tvp5150) i'll try to
make a summary of the current situation and understand what is needed
to finally get it into the main tree instead of having to apply a
dozen patches manually.

The current status of git repositories/branches is:

- main tree: working (i suppose) but no support for bt656 input

- pinchartl/media:
  * omap3isp-omap3isp-next: i think it's in sync with linuxtv master
(for the omap3-isp parts)
  * omap3isp-omap3isp-yuv: like ..next but with some additional format patches

"Floating" patches:

- Deepthy: sent patches (against mainline) to add bt656 support

Laurent made some comments, i haven't seen a v2 to be applied

- Javier: sent patches for tvp5150, currently discussed on
linux-media; possible patches/fixes for omap3-isp

Now what can we all do to converge to a final solution? I think this
is also blocking the possible development/test of missing features,
like the recently-discussed resizer and cropping ones.

Enrico

Re: omap3-isp status

[Javier Martinez Canillas]

On Wed, Oct 5, 2011 at 6:28 PM, Enrico <ebutera@users.berlios.de> wrote:
> Hi all,
>
> since we are all interested in this driver (and tvp5150) i'll try to
> make a summary of the current situation and understand what is needed
> to finally get it into the main tree instead of having to apply a
> dozen patches manually.
>
> The current status of git repositories/branches is:
>
> - main tree: working (i suppose) but no support for bt656 input
>
> - pinchartl/media:
>  * omap3isp-omap3isp-next: i think it's in sync with linuxtv master
> (for the omap3-isp parts)
>  * omap3isp-omap3isp-yuv: like ..next but with some additional format patches
>
> "Floating" patches:
>
> - Deepthy: sent patches (against mainline) to add bt656 support
>
> Laurent made some comments, i haven't seen a v2 to be applied
>
> - Javier: sent patches for tvp5150, currently discussed on
> linux-media; possible patches/fixes for omap3-isp
>

I will find some free time slots to resolve the issues called out by
Sakari, Hans and Mauro and resend the patch-set for the tvp5151.

Also I can send the patches of the modifications I made to the ISP
driver. Right now I'm working on top of Deepthy patches.

I can either send on top of that patch or rebase to mainline, whatever
you think is better for reviewing.

> Now what can we all do to converge to a final solution? I think this
> is also blocking the possible development/test of missing features,
> like the recently-discussed resizer and cropping ones.
>
> Enrico
>

Right now I have a working the tvp5151 with the ISP. I can capture
ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
pipeline is configured automatically with the video standard detected
by the tvp5151. Also, I'm using the CCDC to crop the frames and only
capture the active lines for each standard (576 for PAL and 480 for
NTSC) using the CCDC to crop the image.

Tomorrow we will push our patches to one of our development trees so
you can review it.

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Javier Martinez Canillas]

On Wed, Oct 5, 2011 at 7:43 PM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Wed, Oct 5, 2011 at 6:28 PM, Enrico <ebutera@users.berlios.de> wrote:
>> Hi all,
>>
>> since we are all interested in this driver (and tvp5150) i'll try to
>> make a summary of the current situation and understand what is needed
>> to finally get it into the main tree instead of having to apply a
>> dozen patches manually.
>>
>> The current status of git repositories/branches is:
>>
>> - main tree: working (i suppose) but no support for bt656 input
>>
>> - pinchartl/media:
>>  * omap3isp-omap3isp-next: i think it's in sync with linuxtv master
>> (for the omap3-isp parts)
>>  * omap3isp-omap3isp-yuv: like ..next but with some additional format patches
>>
>> "Floating" patches:
>>
>> - Deepthy: sent patches (against mainline) to add bt656 support
>>
>> Laurent made some comments, i haven't seen a v2 to be applied
>>
>> - Javier: sent patches for tvp5150, currently discussed on
>> linux-media; possible patches/fixes for omap3-isp
>>

Hello,

Since the patches are not against mainline I can't post for reviewing
but can be found in one of our development trees [1]. Comments are
highly appreciated.

The tree is a 2.6.37 that already contain Deepthy patch. I rebased my
changes on top of that to correctly support both BT656 an non-BT656
video data processing.

[1]: http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=shortlog;h=refs/heads/linux-2.6.37.y-next

>
> I will find some free time slots to resolve the issues called out by
> Sakari, Hans and Mauro and resend the patch-set for the tvp5151.
>
> Also I can send the patches of the modifications I made to the ISP
> driver. Right now I'm working on top of Deepthy patches.
>
> I can either send on top of that patch or rebase to mainline, whatever
> you think is better for reviewing.
>
>> Now what can we all do to converge to a final solution? I think this
>> is also blocking the possible development/test of missing features,
>> like the recently-discussed resizer and cropping ones.
>>
>> Enrico
>>
>
> Right now I have a working the tvp5151 with the ISP. I can capture
> ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
> pipeline is configured automatically with the video standard detected
> by the tvp5151. Also, I'm using the CCDC to crop the frames and only
> capture the active lines for each standard (576 for PAL and 480 for
> NTSC) using the CCDC to crop the image.
>

As I told you before video capturing is working for both PAL and NTSC
using standard V4L2 application (i.e: gstreamer) but the video still
shows some motion artifacts. Capturing YUV frames and looking at them
I realized that there does exist a pattern, the sequence 2 frames
correct and 3 frames with interlacing effects always repeats.

Hope it helps.

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Gary Thomas]

On 2011-10-06 01:51, Javier Martinez Canillas wrote:
> On Wed, Oct 5, 2011 at 7:43 PM, Javier Martinez Canillas
> <martinez.javier@gmail.com>  wrote:
>> On Wed, Oct 5, 2011 at 6:28 PM, Enrico<ebutera@users.berlios.de>  wrote:
>>> Hi all,
>>>
>>> since we are all interested in this driver (and tvp5150) i'll try to
>>> make a summary of the current situation and understand what is needed
>>> to finally get it into the main tree instead of having to apply a
>>> dozen patches manually.
>>>
>>> The current status of git repositories/branches is:
>>>
>>> - main tree: working (i suppose) but no support for bt656 input
>>>
>>> - pinchartl/media:
>>>   * omap3isp-omap3isp-next: i think it's in sync with linuxtv master
>>> (for the omap3-isp parts)
>>>   * omap3isp-omap3isp-yuv: like ..next but with some additional format patches
>>>
>>> "Floating" patches:
>>>
>>> - Deepthy: sent patches (against mainline) to add bt656 support
>>>
>>> Laurent made some comments, i haven't seen a v2 to be applied
>>>
>>> - Javier: sent patches for tvp5150, currently discussed on
>>> linux-media; possible patches/fixes for omap3-isp
>>>
>
> Hello,
>
> Since the patches are not against mainline I can't post for reviewing
> but can be found in one of our development trees [1]. Comments are
> highly appreciated.
>
> The tree is a 2.6.37 that already contain Deepthy patch. I rebased my
> changes on top of that to correctly support both BT656 an non-BT656
> video data processing.
>
> [1]: http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=shortlog;h=refs/heads/linux-2.6.37.y-next

Any chance of rebasing these against a more up to date kernel, e.g. 3.2-working
with the patches Laurent sent today?

>>
>> I will find some free time slots to resolve the issues called out by
>> Sakari, Hans and Mauro and resend the patch-set for the tvp5151.
>>
>> Also I can send the patches of the modifications I made to the ISP
>> driver. Right now I'm working on top of Deepthy patches.
>>
>> I can either send on top of that patch or rebase to mainline, whatever
>> you think is better for reviewing.
>>
>>> Now what can we all do to converge to a final solution? I think this
>>> is also blocking the possible development/test of missing features,
>>> like the recently-discussed resizer and cropping ones.
>>>
>>> Enrico
>>>
>>
>> Right now I have a working the tvp5151 with the ISP. I can capture
>> ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
>> pipeline is configured automatically with the video standard detected
>> by the tvp5151. Also, I'm using the CCDC to crop the frames and only
>> capture the active lines for each standard (576 for PAL and 480 for
>> NTSC) using the CCDC to crop the image.
>>
>
> As I told you before video capturing is working for both PAL and NTSC
> using standard V4L2 application (i.e: gstreamer) but the video still
> shows some motion artifacts. Capturing YUV frames and looking at them
> I realized that there does exist a pattern, the sequence 2 frames
> correct and 3 frames with interlacing effects always repeats.

I think I've seen this as well.  Could you provide a short video
which shows the artefacts?

Thanks

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Re: omap3-isp status

[Javier Martinez Canillas]

On Thu, Oct 6, 2011 at 4:29 PM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Thu, Oct 6, 2011 at 4:00 PM, Gary Thomas <gary@mlbassoc.com> wrote:
>> On 2011-10-06 01:51, Javier Martinez Canillas wrote:
>>>
>>> On Wed, Oct 5, 2011 at 7:43 PM, Javier Martinez Canillas
>>> <martinez.javier@gmail.com>  wrote:
>>>>
>>>> On Wed, Oct 5, 2011 at 6:28 PM, Enrico<ebutera@users.berlios.de>  wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> since we are all interested in this driver (and tvp5150) i'll try to
>>>>> make a summary of the current situation and understand what is needed
>>>>> to finally get it into the main tree instead of having to apply a
>>>>> dozen patches manually.
>>>>>
>>>>> The current status of git repositories/branches is:
>>>>>
>>>>> - main tree: working (i suppose) but no support for bt656 input
>>>>>
>>>>> - pinchartl/media:
>>>>>  * omap3isp-omap3isp-next: i think it's in sync with linuxtv master
>>>>> (for the omap3-isp parts)
>>>>>  * omap3isp-omap3isp-yuv: like ..next but with some additional format
>>>>> patches
>>>>>
>>>>> "Floating" patches:
>>>>>
>>>>> - Deepthy: sent patches (against mainline) to add bt656 support
>>>>>
>>>>> Laurent made some comments, i haven't seen a v2 to be applied
>>>>>
>>>>> - Javier: sent patches for tvp5150, currently discussed on
>>>>> linux-media; possible patches/fixes for omap3-isp
>>>>>
>>>
>>> Hello,
>>>
>>> Since the patches are not against mainline I can't post for reviewing
>>> but can be found in one of our development trees [1]. Comments are
>>> highly appreciated.
>>>
>>> The tree is a 2.6.37 that already contain Deepthy patch. I rebased my
>>> changes on top of that to correctly support both BT656 an non-BT656
>>> video data processing.
>>>
>>> [1]:
>>> http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=shortlog;h=refs/heads/linux-2.6.37.y-next
>>
>> Any chance of rebasing these against a more up to date kernel, e.g.
>> 3.2-working
>> with the patches Laurent sent today?
>>
>
> Sure, but I won't have time to do it neither today nor tomorrow. But
> will do it during the weekend.
>
>>>>
>>>> I will find some free time slots to resolve the issues called out by
>>>> Sakari, Hans and Mauro and resend the patch-set for the tvp5151.
>>>>
>>>> Also I can send the patches of the modifications I made to the ISP
>>>> driver. Right now I'm working on top of Deepthy patches.
>>>>
>>>> I can either send on top of that patch or rebase to mainline, whatever
>>>> you think is better for reviewing.
>>>>
>>>>> Now what can we all do to converge to a final solution? I think this
>>>>> is also blocking the possible development/test of missing features,
>>>>> like the recently-discussed resizer and cropping ones.
>>>>>
>>>>> Enrico
>>>>>
>>>>
>>>> Right now I have a working the tvp5151 with the ISP. I can capture
>>>> ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
>>>> pipeline is configured automatically with the video standard detected
>>>> by the tvp5151. Also, I'm using the CCDC to crop the frames and only
>>>> capture the active lines for each standard (576 for PAL and 480 for
>>>> NTSC) using the CCDC to crop the image.
>>>>
>>>
>>> As I told you before video capturing is working for both PAL and NTSC
>>> using standard V4L2 application (i.e: gstreamer) but the video still
>>> shows some motion artifacts. Capturing YUV frames and looking at them
>>> I realized that there does exist a pattern, the sequence 2 frames
>>> correct and 3 frames with interlacing effects always repeats.
>>
>> I think I've seen this as well.  Could you provide a short video
>> which shows the artefacts?
>>
>
> Yes, I've attached a 16-frame video file. It is a PAL-M video
> (720x576) in YUV 4:22 data format. Please let me know if it is OK for
> you.
>

Sorry, I didn't notice the size of the image (13 MB) and got a lot of
rejects from your MTAs. I uploaded the file to my personal github
account [1].

[1]: https://github.com/martinezjavier/omap3isp_tvp5151/blob/master/pal.yuv

Regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Enrico]

On Thu, Oct 6, 2011 at 9:51 AM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> Since the patches are not against mainline I can't post for reviewing
> but can be found in one of our development trees [1]. Comments are
> highly appreciated.
>
> The tree is a 2.6.37 that already contain Deepthy patch. I rebased my
> changes on top of that to correctly support both BT656 an non-BT656
> video data processing.
>
> [1]: http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=shortlog;h=refs/heads/linux-2.6.37.y-next

Some random comments from a quick view at [1]:

- i don't see Deepthy patches, it seems to be based on the
pre-Deepthy-patches driver and fixed (not that this is a bad thing!);
i say this because, like Gary, i'm interested in a possible forward
porting to a more recent kernel so i was searching for a starting
point

- i don't think that adding the "priv" field in v4l2-mediabus.h will
be accepted, and since it is related to the default cropping you added
i think it can be dropped and just let the user choose the appropriate
cropping

- because of the previous point, i think the
PAL(NTSC)_NUM_ACTIVE_LINES can stay to 625(525)

- we really need some comments from someone that is not me, you and Gary

[1]: http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=history;f=drivers/media/video/isp;hb=refs/heads/linux-2.6.37.y-next


>> Right now I have a working the tvp5151 with the ISP. I can capture
>> ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
>> pipeline is configured automatically with the video standard detected
>> by the tvp5151. Also, I'm using the CCDC to crop the frames and only
>> capture the active lines for each standard (576 for PAL and 480 for
>> NTSC) using the CCDC to crop the image.
>>
>
> As I told you before video capturing is working for both PAL and NTSC
> using standard V4L2 application (i.e: gstreamer) but the video still
> shows some motion artifacts. Capturing YUV frames and looking at them
> I realized that there does exist a pattern, the sequence 2 frames
> correct and 3 frames with interlacing effects always repeats.

Yes i've seen that too, i was planning to do some tests when things
will settle down.

Enrico

Re: omap3-isp status

[Gary Thomas]

On 2011-10-06 08:50, Javier Martinez Canillas wrote:
> On Thu, Oct 6, 2011 at 4:29 PM, Javier Martinez Canillas
> <martinez.javier@gmail.com>  wrote:
>> On Thu, Oct 6, 2011 at 4:00 PM, Gary Thomas<gary@mlbassoc.com>  wrote:
>>> On 2011-10-06 01:51, Javier Martinez Canillas wrote:
>>>>
>>>> On Wed, Oct 5, 2011 at 7:43 PM, Javier Martinez Canillas
>>>> <martinez.javier@gmail.com>    wrote:
>>>>>
>>>>> On Wed, Oct 5, 2011 at 6:28 PM, Enrico<ebutera@users.berlios.de>    wrote:
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> since we are all interested in this driver (and tvp5150) i'll try to
>>>>>> make a summary of the current situation and understand what is needed
>>>>>> to finally get it into the main tree instead of having to apply a
>>>>>> dozen patches manually.
>>>>>>
>>>>>> The current status of git repositories/branches is:
>>>>>>
>>>>>> - main tree: working (i suppose) but no support for bt656 input
>>>>>>
>>>>>> - pinchartl/media:
>>>>>>   * omap3isp-omap3isp-next: i think it's in sync with linuxtv master
>>>>>> (for the omap3-isp parts)
>>>>>>   * omap3isp-omap3isp-yuv: like ..next but with some additional format
>>>>>> patches
>>>>>>
>>>>>> "Floating" patches:
>>>>>>
>>>>>> - Deepthy: sent patches (against mainline) to add bt656 support
>>>>>>
>>>>>> Laurent made some comments, i haven't seen a v2 to be applied
>>>>>>
>>>>>> - Javier: sent patches for tvp5150, currently discussed on
>>>>>> linux-media; possible patches/fixes for omap3-isp
>>>>>>
>>>>
>>>> Hello,
>>>>
>>>> Since the patches are not against mainline I can't post for reviewing
>>>> but can be found in one of our development trees [1]. Comments are
>>>> highly appreciated.
>>>>
>>>> The tree is a 2.6.37 that already contain Deepthy patch. I rebased my
>>>> changes on top of that to correctly support both BT656 an non-BT656
>>>> video data processing.
>>>>
>>>> [1]:
>>>> http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=shortlog;h=refs/heads/linux-2.6.37.y-next
>>>
>>> Any chance of rebasing these against a more up to date kernel, e.g.
>>> 3.2-working
>>> with the patches Laurent sent today?
>>>
>>
>> Sure, but I won't have time to do it neither today nor tomorrow. But
>> will do it during the weekend.
>>
>>>>>
>>>>> I will find some free time slots to resolve the issues called out by
>>>>> Sakari, Hans and Mauro and resend the patch-set for the tvp5151.
>>>>>
>>>>> Also I can send the patches of the modifications I made to the ISP
>>>>> driver. Right now I'm working on top of Deepthy patches.
>>>>>
>>>>> I can either send on top of that patch or rebase to mainline, whatever
>>>>> you think is better for reviewing.
>>>>>
>>>>>> Now what can we all do to converge to a final solution? I think this
>>>>>> is also blocking the possible development/test of missing features,
>>>>>> like the recently-discussed resizer and cropping ones.
>>>>>>
>>>>>> Enrico
>>>>>>
>>>>>
>>>>> Right now I have a working the tvp5151 with the ISP. I can capture
>>>>> ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
>>>>> pipeline is configured automatically with the video standard detected
>>>>> by the tvp5151. Also, I'm using the CCDC to crop the frames and only
>>>>> capture the active lines for each standard (576 for PAL and 480 for
>>>>> NTSC) using the CCDC to crop the image.
>>>>>
>>>>
>>>> As I told you before video capturing is working for both PAL and NTSC
>>>> using standard V4L2 application (i.e: gstreamer) but the video still
>>>> shows some motion artifacts. Capturing YUV frames and looking at them
>>>> I realized that there does exist a pattern, the sequence 2 frames
>>>> correct and 3 frames with interlacing effects always repeats.
>>>
>>> I think I've seen this as well.  Could you provide a short video
>>> which shows the artefacts?
>>>
>>
>> Yes, I've attached a 16-frame video file. It is a PAL-M video
>> (720x576) in YUV 4:22 data format. Please let me know if it is OK for
>> you.
>>
>
> Sorry, I didn't notice the size of the image (13 MB) and got a lot of
> rejects from your MTAs. I uploaded the file to my personal github
> account [1].
>
> [1]: https://github.com/martinezjavier/omap3isp_tvp5151/blob/master/pal.yuv

Very interesting.  What was your source (camera type, etc)?
How are you looking (or extracting) individual frames for analysis?

I see much the same sort of artefacts as you are.  An example is at
   http://www.mlbassoc.com/misc/untitled.m2t
This is a little example I put together using kdenlive.  The first segment
is the raw video from my camera, imported via USB.  The second is roughly
the same video captured using my OMAP board and converted to MP4 on the fly
by this command:
   ffmpeg -r 30/1 -pix_fmt uyvy422 -s 720x524 -f video4linux2 -i /dev/video2 -qscale 1 -f mp4 test1.mp4
I think there are some aspect ratio issues with these but what bothers me
the most is how much the captured data tears whenever there is a lot of
motion in the video.

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Re: omap3-isp status

[Javier Martinez Canillas]

On Thu, Oct 6, 2011 at 5:25 PM, Enrico <ebutera@users.berlios.de> wrote:
> On Thu, Oct 6, 2011 at 9:51 AM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> Since the patches are not against mainline I can't post for reviewing
>> but can be found in one of our development trees [1]. Comments are
>> highly appreciated.
>>
>> The tree is a 2.6.37 that already contain Deepthy patch. I rebased my
>> changes on top of that to correctly support both BT656 an non-BT656
>> video data processing.
>>
>> [1]: http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=shortlog;h=refs/heads/linux-2.6.37.y-next
>
> Some random comments from a quick view at [1]:
>
> - i don't see Deepthy patches, it seems to be based on the
> pre-Deepthy-patches driver and fixed (not that this is a bad thing!);
> i say this because, like Gary, i'm interested in a possible forward
> porting to a more recent kernel so i was searching for a starting
> point
>

I didn't know there was a more recent version of Deepthy patches,
Since they are not yet in mainline we should decide if we work on top
of that or on top of mainline. Deepthy patches are very good to
separate bt656 and non-bt656 execution inside the ISP, also add a
platform data variable to decide which mode has to be used.

But reading the documentation and from my experimental validation I
think that there are a few things that can be improved.

First the assumption that we can use FLDSTAT to check if a frame is
ODD or EVEN I find to not always be true. Also I don't know who sets
this value since in the TRM always talks as it is only used with
discrete syncs.

Also, I don't think that we should change the ISP CCDC configuration
inside the VD0 interrupt handler. Since the shadowed registers only
can be accessed during a frame processing, or more formally the new
values are taken at the beginning of a frame execution.

By the time we change for example the output address memory for the
next buffer in the VD0 handler, the next frame is already being
processed so that value won't be used for the CCDC until that frame
finish. So It is not behaving as the code expect, since for 3 frames
the CCDC output memory address will be the same.

That is why I move most of the logic to the VD1 interrupt since there
the current frame didn't finish yet and we can configure the CCDC for
the next frame.

But to do that the buffer for the next frame and the releasing of the
last buffer can't happen simultaneously, that is why I decouple these
two actions.

Again, this is my own observations and what I understood from the TRM
and I could be wrong.

> - i don't think that adding the "priv" field in v4l2-mediabus.h will
> be accepted, and since it is related to the default cropping you added
> i think it can be dropped and just let the user choose the appropriate
> cropping
>

Yes, probably is too much of a hack, but I didn't know of another way
that the subdev could report to the ISP of the standard and since
v4l2_pix_format has also a priv field, I think it could be at least a
temporary solution (remember that we want this to work first and then
we plan to do it right for upstream submission).

> - because of the previous point, i think the
> PAL(NTSC)_NUM_ACTIVE_LINES can stay to 625(525)
>
> - we really need some comments from someone that is not me, you and Gary
>
> [1]: http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=history;f=drivers/media/video/isp;hb=refs/heads/linux-2.6.37.y-next
>
>
>>> Right now I have a working the tvp5151 with the ISP. I can capture
>>> ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
>>> pipeline is configured automatically with the video standard detected
>>> by the tvp5151. Also, I'm using the CCDC to crop the frames and only
>>> capture the active lines for each standard (576 for PAL and 480 for
>>> NTSC) using the CCDC to crop the image.
>>>
>>
>> As I told you before video capturing is working for both PAL and NTSC
>> using standard V4L2 application (i.e: gstreamer) but the video still
>> shows some motion artifacts. Capturing YUV frames and looking at them
>> I realized that there does exist a pattern, the sequence 2 frames
>> correct and 3 frames with interlacing effects always repeats.
>
> Yes i've seen that too, i was planning to do some tests when things
> will settle down.
>
> Enrico
>

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Javier Martinez Canillas]

On Thu, Oct 6, 2011 at 5:47 PM, Gary Thomas <gary@mlbassoc.com> wrote:
> On 2011-10-06 08:50, Javier Martinez Canillas wrote:
>>
>> On Thu, Oct 6, 2011 at 4:29 PM, Javier Martinez Canillas
>> <martinez.javier@gmail.com>  wrote:
>>>
>>> On Thu, Oct 6, 2011 at 4:00 PM, Gary Thomas<gary@mlbassoc.com>  wrote:
>>>>
>>>> On 2011-10-06 01:51, Javier Martinez Canillas wrote:
>>>>>
>>>>> On Wed, Oct 5, 2011 at 7:43 PM, Javier Martinez Canillas
>>>>> <martinez.javier@gmail.com>    wrote:
>>>>>>
>>>>>> On Wed, Oct 5, 2011 at 6:28 PM, Enrico<ebutera@users.berlios.de>
>>>>>>  wrote:
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> since we are all interested in this driver (and tvp5150) i'll try to
>>>>>>> make a summary of the current situation and understand what is needed
>>>>>>> to finally get it into the main tree instead of having to apply a
>>>>>>> dozen patches manually.
>>>>>>>
>>>>>>> The current status of git repositories/branches is:
>>>>>>>
>>>>>>> - main tree: working (i suppose) but no support for bt656 input
>>>>>>>
>>>>>>> - pinchartl/media:
>>>>>>>  * omap3isp-omap3isp-next: i think it's in sync with linuxtv master
>>>>>>> (for the omap3-isp parts)
>>>>>>>  * omap3isp-omap3isp-yuv: like ..next but with some additional format
>>>>>>> patches
>>>>>>>
>>>>>>> "Floating" patches:
>>>>>>>
>>>>>>> - Deepthy: sent patches (against mainline) to add bt656 support
>>>>>>>
>>>>>>> Laurent made some comments, i haven't seen a v2 to be applied
>>>>>>>
>>>>>>> - Javier: sent patches for tvp5150, currently discussed on
>>>>>>> linux-media; possible patches/fixes for omap3-isp
>>>>>>>
>>>>>
>>>>> Hello,
>>>>>
>>>>> Since the patches are not against mainline I can't post for reviewing
>>>>> but can be found in one of our development trees [1]. Comments are
>>>>> highly appreciated.
>>>>>
>>>>> The tree is a 2.6.37 that already contain Deepthy patch. I rebased my
>>>>> changes on top of that to correctly support both BT656 an non-BT656
>>>>> video data processing.
>>>>>
>>>>> [1]:
>>>>>
>>>>> http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=shortlog;h=refs/heads/linux-2.6.37.y-next
>>>>
>>>> Any chance of rebasing these against a more up to date kernel, e.g.
>>>> 3.2-working
>>>> with the patches Laurent sent today?
>>>>
>>>
>>> Sure, but I won't have time to do it neither today nor tomorrow. But
>>> will do it during the weekend.
>>>
>>>>>>
>>>>>> I will find some free time slots to resolve the issues called out by
>>>>>> Sakari, Hans and Mauro and resend the patch-set for the tvp5151.
>>>>>>
>>>>>> Also I can send the patches of the modifications I made to the ISP
>>>>>> driver. Right now I'm working on top of Deepthy patches.
>>>>>>
>>>>>> I can either send on top of that patch or rebase to mainline, whatever
>>>>>> you think is better for reviewing.
>>>>>>
>>>>>>> Now what can we all do to converge to a final solution? I think this
>>>>>>> is also blocking the possible development/test of missing features,
>>>>>>> like the recently-discussed resizer and cropping ones.
>>>>>>>
>>>>>>> Enrico
>>>>>>>
>>>>>>
>>>>>> Right now I have a working the tvp5151 with the ISP. I can capture
>>>>>> ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
>>>>>> pipeline is configured automatically with the video standard detected
>>>>>> by the tvp5151. Also, I'm using the CCDC to crop the frames and only
>>>>>> capture the active lines for each standard (576 for PAL and 480 for
>>>>>> NTSC) using the CCDC to crop the image.
>>>>>>
>>>>>
>>>>> As I told you before video capturing is working for both PAL and NTSC
>>>>> using standard V4L2 application (i.e: gstreamer) but the video still
>>>>> shows some motion artifacts. Capturing YUV frames and looking at them
>>>>> I realized that there does exist a pattern, the sequence 2 frames
>>>>> correct and 3 frames with interlacing effects always repeats.
>>>>
>>>> I think I've seen this as well.  Could you provide a short video
>>>> which shows the artefacts?
>>>>
>>>
>>> Yes, I've attached a 16-frame video file. It is a PAL-M video
>>> (720x576) in YUV 4:22 data format. Please let me know if it is OK for
>>> you.
>>>
>>
>> Sorry, I didn't notice the size of the image (13 MB) and got a lot of
>> rejects from your MTAs. I uploaded the file to my personal github
>> account [1].
>>
>> [1]:
>> https://github.com/martinezjavier/omap3isp_tvp5151/blob/master/pal.yuv
>
> Very interesting.  What was your source (camera type, etc)?

A samsung HD video camera connected to the TVP with its RCA video
connector. But the artifact its independent of the analog input data,
I've tried with an Sony Cybershot camera and other input sources.

> How are you looking (or extracting) individual frames for analysis?
>

I'm using gstreamer to capture RAW YUV data and MPEG encoded video
using the DSP.

This are my pipelines:

YUV:

gst-launch-0.10 -v v4l2src device=/dev/video2 queue-size=16
num-buffers=16 ! video/x-raw-yuv,format=\(fourcc\)UYVY ! filesink
location=capture.out

MPEG:

gst-launch-0.10 -v v4l2src device=/dev/video2 queue-size=8 !
video/x-raw-yuv,format=\(fourcc\)UYVY ! TIVidenc1 codecName=mpeg4enc
engineName=codecServer ! qtmux ! filesink location=capture.m4v

> I see much the same sort of artefacts as you are.  An example is at
>  http://www.mlbassoc.com/misc/untitled.m2t
> This is a little example I put together using kdenlive.  The first segment
> is the raw video from my camera, imported via USB.  The second is roughly
> the same video captured using my OMAP board and converted to MP4 on the fly
> by this command:
>  ffmpeg -r 30/1 -pix_fmt uyvy422 -s 720x524 -f video4linux2 -i /dev/video2
> -qscale 1 -f mp4 test1.mp4
> I think there are some aspect ratio issues with these but what bothers me
> the most is how much the captured data tears whenever there is a lot of
> motion in the video.
>
> --
> ------------------------------------------------------------
> Gary Thomas                 |  Consulting for the
> MLB Associates              |    Embedded world
> ------------------------------------------------------------
>

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Enrico]

On Thu, Oct 6, 2011 at 6:05 PM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Thu, Oct 6, 2011 at 5:25 PM, Enrico <ebutera@users.berlios.de> wrote:
>> - i don't see Deepthy patches, it seems to be based on the
>> pre-Deepthy-patches driver and fixed (not that this is a bad thing!);
>> i say this because, like Gary, i'm interested in a possible forward
>> porting to a more recent kernel so i was searching for a starting
>> point
>>
>
> I didn't know there was a more recent version of Deepthy patches,
> Since they are not yet in mainline we should decide if we work on top
> of that or on top of mainline. Deepthy patches are very good to
> separate bt656 and non-bt656 execution inside the ISP, also add a
> platform data variable to decide which mode has to be used.
>
> But reading the documentation and from my experimental validation I
> think that there are a few things that can be improved.
>
> First the assumption that we can use FLDSTAT to check if a frame is
> ODD or EVEN I find to not always be true. Also I don't know who sets
> this value since in the TRM always talks as it is only used with
> discrete syncs.


Yes about FLDSTAT i noticed the same thing. And that's why we need
someone that knows the ISP better to help us....


> Also, I don't think that we should change the ISP CCDC configuration
> inside the VD0 interrupt handler. Since the shadowed registers only
> can be accessed during a frame processing, or more formally the new
> values are taken at the beginning of a frame execution.
>
> By the time we change for example the output address memory for the
> next buffer in the VD0 handler, the next frame is already being
> processed so that value won't be used for the CCDC until that frame
> finish. So It is not behaving as the code expect, since for 3 frames
> the CCDC output memory address will be the same.
>
> That is why I move most of the logic to the VD1 interrupt since there
> the current frame didn't finish yet and we can configure the CCDC for
> the next frame.
>
> But to do that the buffer for the next frame and the releasing of the
> last buffer can't happen simultaneously, that is why I decouple these
> two actions.
>
> Again, this is my own observations and what I understood from the TRM
> and I could be wrong.


I can't comment on that, i hope Laurent or Deepthy will join the discussion...


>> - i don't think that adding the "priv" field in v4l2-mediabus.h will
>> be accepted, and since it is related to the default cropping you added
>> i think it can be dropped and just let the user choose the appropriate
>> cropping
>>
>
> Yes, probably is too much of a hack, but I didn't know of another way
> that the subdev could report to the ISP of the standard and since
> v4l2_pix_format has also a priv field, I think it could be at least a
> temporary solution (remember that we want this to work first and then
> we plan to do it right for upstream submission).


...and my hope continues here.


Enrico

Re: omap3-isp status

[Javier Martinez Canillas]

On Fri, Oct 7, 2011 at 10:54 AM, Enrico <ebutera@users.berlios.de> wrote:
> On Thu, Oct 6, 2011 at 6:05 PM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> On Thu, Oct 6, 2011 at 5:25 PM, Enrico <ebutera@users.berlios.de> wrote:
>>> - i don't see Deepthy patches, it seems to be based on the
>>> pre-Deepthy-patches driver and fixed (not that this is a bad thing!);
>>> i say this because, like Gary, i'm interested in a possible forward
>>> porting to a more recent kernel so i was searching for a starting
>>> point
>>>
>>
>> I didn't know there was a more recent version of Deepthy patches,
>> Since they are not yet in mainline we should decide if we work on top
>> of that or on top of mainline. Deepthy patches are very good to
>> separate bt656 and non-bt656 execution inside the ISP, also add a
>> platform data variable to decide which mode has to be used.
>>
>> But reading the documentation and from my experimental validation I
>> think that there are a few things that can be improved.
>>
>> First the assumption that we can use FLDSTAT to check if a frame is
>> ODD or EVEN I find to not always be true. Also I don't know who sets
>> this value since in the TRM always talks as it is only used with
>> discrete syncs.
>
>
> Yes about FLDSTAT i noticed the same thing. And that's why we need
> someone that knows the ISP better to help us....
>

Great, good to know that I'm not the only one that noticed this behavior.

>
>> Also, I don't think that we should change the ISP CCDC configuration
>> inside the VD0 interrupt handler. Since the shadowed registers only
>> can be accessed during a frame processing, or more formally the new
>> values are taken at the beginning of a frame execution.
>>
>> By the time we change for example the output address memory for the
>> next buffer in the VD0 handler, the next frame is already being
>> processed so that value won't be used for the CCDC until that frame
>> finish. So It is not behaving as the code expect, since for 3 frames
>> the CCDC output memory address will be the same.
>>
>> That is why I move most of the logic to the VD1 interrupt since there
>> the current frame didn't finish yet and we can configure the CCDC for
>> the next frame.
>>
>> But to do that the buffer for the next frame and the releasing of the
>> last buffer can't happen simultaneously, that is why I decouple these
>> two actions.
>>
>> Again, this is my own observations and what I understood from the TRM
>> and I could be wrong.
>
>
> I can't comment on that, i hope Laurent or Deepthy will join the discussion...
>
>

I second you on that, we need someone who knows the ISP better than we
do. I have to fix this anyway, so it is better if I can do it the
right way and the code gos upstream, so we don't have to internally
maintain a separate patch-set and forward port for each kernel release
we do.

>>> - i don't think that adding the "priv" field in v4l2-mediabus.h will
>>> be accepted, and since it is related to the default cropping you added
>>> i think it can be dropped and just let the user choose the appropriate
>>> cropping
>>>
>>
>> Yes, probably is too much of a hack, but I didn't know of another way
>> that the subdev could report to the ISP of the standard and since
>> v4l2_pix_format has also a priv field, I think it could be at least a
>> temporary solution (remember that we want this to work first and then
>> we plan to do it right for upstream submission).
>
>
> ...and my hope continues here.
>
>
> Enrico
>

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Gary Thomas]

On 2011-10-06 10:11, Javier Martinez Canillas wrote:
> On Thu, Oct 6, 2011 at 5:47 PM, Gary Thomas<gary@mlbassoc.com>  wrote:
>> On 2011-10-06 08:50, Javier Martinez Canillas wrote:
>>>
>>> On Thu, Oct 6, 2011 at 4:29 PM, Javier Martinez Canillas
>>> <martinez.javier@gmail.com>    wrote:
>>>>
>>>> On Thu, Oct 6, 2011 at 4:00 PM, Gary Thomas<gary@mlbassoc.com>    wrote:
>>>>>
>>>>> On 2011-10-06 01:51, Javier Martinez Canillas wrote:
>>>>>>
>>>>>> On Wed, Oct 5, 2011 at 7:43 PM, Javier Martinez Canillas
>>>>>> <martinez.javier@gmail.com>      wrote:
>>>>>>>
>>>>>>> On Wed, Oct 5, 2011 at 6:28 PM, Enrico<ebutera@users.berlios.de>
>>>>>>>   wrote:
>>>>>>>>
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> since we are all interested in this driver (and tvp5150) i'll try to
>>>>>>>> make a summary of the current situation and understand what is needed
>>>>>>>> to finally get it into the main tree instead of having to apply a
>>>>>>>> dozen patches manually.
>>>>>>>>
>>>>>>>> The current status of git repositories/branches is:
>>>>>>>>
>>>>>>>> - main tree: working (i suppose) but no support for bt656 input
>>>>>>>>
>>>>>>>> - pinchartl/media:
>>>>>>>>   * omap3isp-omap3isp-next: i think it's in sync with linuxtv master
>>>>>>>> (for the omap3-isp parts)
>>>>>>>>   * omap3isp-omap3isp-yuv: like ..next but with some additional format
>>>>>>>> patches
>>>>>>>>
>>>>>>>> "Floating" patches:
>>>>>>>>
>>>>>>>> - Deepthy: sent patches (against mainline) to add bt656 support
>>>>>>>>
>>>>>>>> Laurent made some comments, i haven't seen a v2 to be applied
>>>>>>>>
>>>>>>>> - Javier: sent patches for tvp5150, currently discussed on
>>>>>>>> linux-media; possible patches/fixes for omap3-isp
>>>>>>>>
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> Since the patches are not against mainline I can't post for reviewing
>>>>>> but can be found in one of our development trees [1]. Comments are
>>>>>> highly appreciated.
>>>>>>
>>>>>> The tree is a 2.6.37 that already contain Deepthy patch. I rebased my
>>>>>> changes on top of that to correctly support both BT656 an non-BT656
>>>>>> video data processing.
>>>>>>
>>>>>> [1]:
>>>>>>
>>>>>> http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=shortlog;h=refs/heads/linux-2.6.37.y-next
>>>>>
>>>>> Any chance of rebasing these against a more up to date kernel, e.g.
>>>>> 3.2-working
>>>>> with the patches Laurent sent today?
>>>>>
>>>>
>>>> Sure, but I won't have time to do it neither today nor tomorrow. But
>>>> will do it during the weekend.
>>>>
>>>>>>>
>>>>>>> I will find some free time slots to resolve the issues called out by
>>>>>>> Sakari, Hans and Mauro and resend the patch-set for the tvp5151.
>>>>>>>
>>>>>>> Also I can send the patches of the modifications I made to the ISP
>>>>>>> driver. Right now I'm working on top of Deepthy patches.
>>>>>>>
>>>>>>> I can either send on top of that patch or rebase to mainline, whatever
>>>>>>> you think is better for reviewing.
>>>>>>>
>>>>>>>> Now what can we all do to converge to a final solution? I think this
>>>>>>>> is also blocking the possible development/test of missing features,
>>>>>>>> like the recently-discussed resizer and cropping ones.
>>>>>>>>
>>>>>>>> Enrico
>>>>>>>>
>>>>>>>
>>>>>>> Right now I have a working the tvp5151 with the ISP. I can capture
>>>>>>> ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
>>>>>>> pipeline is configured automatically with the video standard detected
>>>>>>> by the tvp5151. Also, I'm using the CCDC to crop the frames and only
>>>>>>> capture the active lines for each standard (576 for PAL and 480 for
>>>>>>> NTSC) using the CCDC to crop the image.
>>>>>>>
>>>>>>
>>>>>> As I told you before video capturing is working for both PAL and NTSC
>>>>>> using standard V4L2 application (i.e: gstreamer) but the video still
>>>>>> shows some motion artifacts. Capturing YUV frames and looking at them
>>>>>> I realized that there does exist a pattern, the sequence 2 frames
>>>>>> correct and 3 frames with interlacing effects always repeats.
>>>>>
>>>>> I think I've seen this as well.  Could you provide a short video
>>>>> which shows the artefacts?
>>>>>
>>>>
>>>> Yes, I've attached a 16-frame video file. It is a PAL-M video
>>>> (720x576) in YUV 4:22 data format. Please let me know if it is OK for
>>>> you.
>>>>
>>>
>>> Sorry, I didn't notice the size of the image (13 MB) and got a lot of
>>> rejects from your MTAs. I uploaded the file to my personal github
>>> account [1].
>>>
>>> [1]:
>>> https://github.com/martinezjavier/omap3isp_tvp5151/blob/master/pal.yuv
>>
>> Very interesting.  What was your source (camera type, etc)?
>
> A samsung HD video camera connected to the TVP with its RCA video
> connector. But the artifact its independent of the analog input data,
> I've tried with an Sony Cybershot camera and other input sources.
>
>> How are you looking (or extracting) individual frames for analysis?
>>
>
> I'm using gstreamer to capture RAW YUV data and MPEG encoded video
> using the DSP.
>
> This are my pipelines:
>
> YUV:
>
> gst-launch-0.10 -v v4l2src device=/dev/video2 queue-size=16
> num-buffers=16 ! video/x-raw-yuv,format=\(fourcc\)UYVY ! filesink
> location=capture.out
>
> MPEG:
>
> gst-launch-0.10 -v v4l2src device=/dev/video2 queue-size=8 !
> video/x-raw-yuv,format=\(fourcc\)UYVY ! TIVidenc1 codecName=mpeg4enc
> engineName=codecServer ! qtmux ! filesink location=capture.m4v
>
>> I see much the same sort of artefacts as you are.  An example is at
>>   http://www.mlbassoc.com/misc/untitled.m2t
>> This is a little example I put together using kdenlive.  The first segment
>> is the raw video from my camera, imported via USB.  The second is roughly
>> the same video captured using my OMAP board and converted to MP4 on the fly
>> by this command:
>>   ffmpeg -r 30/1 -pix_fmt uyvy422 -s 720x524 -f video4linux2 -i /dev/video2
>> -qscale 1 -f mp4 test1.mp4
>> I think there are some aspect ratio issues with these but what bothers me
>> the most is how much the captured data tears whenever there is a lot of
>> motion in the video.

I figured out how to split your raw video into individual frames.  The problems
don't look like only interlace issues to me.  Take a look at
   http://www.mlbassoc.com/misc/UYVY_examples/PAL_from_JavierMartinezCanillas/
especially image #9 which shows some very serious ghosting.

I see the same behaviour here and it bothers me quite a lot.  I do hope that
we can figure out what's causing it - we have a number of customers that are
wanting to do analogue video capture using the OMAP+TVP5150, so it's pretty
important to us.

Thanks for your time

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Re: omap3-isp status

[Javier Martinez Canillas]

On Fri, Oct 7, 2011 at 11:34 AM, Gary Thomas <gary@mlbassoc.com> wrote:
> On 2011-10-06 10:11, Javier Martinez Canillas wrote:
>>
>> On Thu, Oct 6, 2011 at 5:47 PM, Gary Thomas<gary@mlbassoc.com>  wrote:
>>>
>>> On 2011-10-06 08:50, Javier Martinez Canillas wrote:
>>>>
>>>> On Thu, Oct 6, 2011 at 4:29 PM, Javier Martinez Canillas
>>>> <martinez.javier@gmail.com>    wrote:
>>>>>
>>>>> On Thu, Oct 6, 2011 at 4:00 PM, Gary Thomas<gary@mlbassoc.com>
>>>>>  wrote:
>>>>>>
>>>>>> On 2011-10-06 01:51, Javier Martinez Canillas wrote:
>>>>>>>
>>>>>>> On Wed, Oct 5, 2011 at 7:43 PM, Javier Martinez Canillas
>>>>>>> <martinez.javier@gmail.com>      wrote:
>>>>>>>>
>>>>>>>> On Wed, Oct 5, 2011 at 6:28 PM, Enrico<ebutera@users.berlios.de>
>>>>>>>>  wrote:
>>>>>>>>>
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> since we are all interested in this driver (and tvp5150) i'll try
>>>>>>>>> to
>>>>>>>>> make a summary of the current situation and understand what is
>>>>>>>>> needed
>>>>>>>>> to finally get it into the main tree instead of having to apply a
>>>>>>>>> dozen patches manually.
>>>>>>>>>
>>>>>>>>> The current status of git repositories/branches is:
>>>>>>>>>
>>>>>>>>> - main tree: working (i suppose) but no support for bt656 input
>>>>>>>>>
>>>>>>>>> - pinchartl/media:
>>>>>>>>>  * omap3isp-omap3isp-next: i think it's in sync with linuxtv master
>>>>>>>>> (for the omap3-isp parts)
>>>>>>>>>  * omap3isp-omap3isp-yuv: like ..next but with some additional
>>>>>>>>> format
>>>>>>>>> patches
>>>>>>>>>
>>>>>>>>> "Floating" patches:
>>>>>>>>>
>>>>>>>>> - Deepthy: sent patches (against mainline) to add bt656 support
>>>>>>>>>
>>>>>>>>> Laurent made some comments, i haven't seen a v2 to be applied
>>>>>>>>>
>>>>>>>>> - Javier: sent patches for tvp5150, currently discussed on
>>>>>>>>> linux-media; possible patches/fixes for omap3-isp
>>>>>>>>>
>>>>>>>
>>>>>>> Hello,
>>>>>>>
>>>>>>> Since the patches are not against mainline I can't post for reviewing
>>>>>>> but can be found in one of our development trees [1]. Comments are
>>>>>>> highly appreciated.
>>>>>>>
>>>>>>> The tree is a 2.6.37 that already contain Deepthy patch. I rebased my
>>>>>>> changes on top of that to correctly support both BT656 an non-BT656
>>>>>>> video data processing.
>>>>>>>
>>>>>>> [1]:
>>>>>>>
>>>>>>>
>>>>>>> http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=shortlog;h=refs/heads/linux-2.6.37.y-next
>>>>>>
>>>>>> Any chance of rebasing these against a more up to date kernel, e.g.
>>>>>> 3.2-working
>>>>>> with the patches Laurent sent today?
>>>>>>
>>>>>
>>>>> Sure, but I won't have time to do it neither today nor tomorrow. But
>>>>> will do it during the weekend.
>>>>>
>>>>>>>>
>>>>>>>> I will find some free time slots to resolve the issues called out by
>>>>>>>> Sakari, Hans and Mauro and resend the patch-set for the tvp5151.
>>>>>>>>
>>>>>>>> Also I can send the patches of the modifications I made to the ISP
>>>>>>>> driver. Right now I'm working on top of Deepthy patches.
>>>>>>>>
>>>>>>>> I can either send on top of that patch or rebase to mainline,
>>>>>>>> whatever
>>>>>>>> you think is better for reviewing.
>>>>>>>>
>>>>>>>>> Now what can we all do to converge to a final solution? I think
>>>>>>>>> this
>>>>>>>>> is also blocking the possible development/test of missing features,
>>>>>>>>> like the recently-discussed resizer and cropping ones.
>>>>>>>>>
>>>>>>>>> Enrico
>>>>>>>>>
>>>>>>>>
>>>>>>>> Right now I have a working the tvp5151 with the ISP. I can capture
>>>>>>>> ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
>>>>>>>> pipeline is configured automatically with the video standard
>>>>>>>> detected
>>>>>>>> by the tvp5151. Also, I'm using the CCDC to crop the frames and only
>>>>>>>> capture the active lines for each standard (576 for PAL and 480 for
>>>>>>>> NTSC) using the CCDC to crop the image.
>>>>>>>>
>>>>>>>
>>>>>>> As I told you before video capturing is working for both PAL and NTSC
>>>>>>> using standard V4L2 application (i.e: gstreamer) but the video still
>>>>>>> shows some motion artifacts. Capturing YUV frames and looking at them
>>>>>>> I realized that there does exist a pattern, the sequence 2 frames
>>>>>>> correct and 3 frames with interlacing effects always repeats.
>>>>>>
>>>>>> I think I've seen this as well.  Could you provide a short video
>>>>>> which shows the artefacts?
>>>>>>
>>>>>
>>>>> Yes, I've attached a 16-frame video file. It is a PAL-M video
>>>>> (720x576) in YUV 4:22 data format. Please let me know if it is OK for
>>>>> you.
>>>>>
>>>>
>>>> Sorry, I didn't notice the size of the image (13 MB) and got a lot of
>>>> rejects from your MTAs. I uploaded the file to my personal github
>>>> account [1].
>>>>
>>>> [1]:
>>>> https://github.com/martinezjavier/omap3isp_tvp5151/blob/master/pal.yuv
>>>
>>> Very interesting.  What was your source (camera type, etc)?
>>
>> A samsung HD video camera connected to the TVP with its RCA video
>> connector. But the artifact its independent of the analog input data,
>> I've tried with an Sony Cybershot camera and other input sources.
>>
>>> How are you looking (or extracting) individual frames for analysis?
>>>
>>
>> I'm using gstreamer to capture RAW YUV data and MPEG encoded video
>> using the DSP.
>>
>> This are my pipelines:
>>
>> YUV:
>>
>> gst-launch-0.10 -v v4l2src device=/dev/video2 queue-size=16
>> num-buffers=16 ! video/x-raw-yuv,format=\(fourcc\)UYVY ! filesink
>> location=capture.out
>>
>> MPEG:
>>
>> gst-launch-0.10 -v v4l2src device=/dev/video2 queue-size=8 !
>> video/x-raw-yuv,format=\(fourcc\)UYVY ! TIVidenc1 codecName=mpeg4enc
>> engineName=codecServer ! qtmux ! filesink location=capture.m4v
>>
>>> I see much the same sort of artefacts as you are.  An example is at
>>>  http://www.mlbassoc.com/misc/untitled.m2t
>>> This is a little example I put together using kdenlive.  The first
>>> segment
>>> is the raw video from my camera, imported via USB.  The second is roughly
>>> the same video captured using my OMAP board and converted to MP4 on the
>>> fly
>>> by this command:
>>>  ffmpeg -r 30/1 -pix_fmt uyvy422 -s 720x524 -f video4linux2 -i
>>> /dev/video2
>>> -qscale 1 -f mp4 test1.mp4
>>> I think there are some aspect ratio issues with these but what bothers me
>>> the most is how much the captured data tears whenever there is a lot of
>>> motion in the video.
>
> I figured out how to split your raw video into individual frames.  The
> problems
> don't look like only interlace issues to me.  Take a look at
>  http://www.mlbassoc.com/misc/UYVY_examples/PAL_from_JavierMartinezCanillas/
> especially image #9 which shows some very serious ghosting.
>

Yes, I don't know if this is because I'm not copying the sub-frame in
the correct buffer or another.

> I see the same behaviour here and it bothers me quite a lot.  I do hope that
> we can figure out what's causing it - we have a number of customers that are
> wanting to do analogue video capture using the OMAP+TVP5150, so it's pretty
> important to us.
>
> Thanks for your time

Yes, we are in the same situation. I did my best to fix this but I
couldn't. I minimized the effect with those changes but there still
exists.

I hope as Enrico says that Laurent, Sakari (who im cc'ing here) or
Deepthy that know better the ISP can enter the discussion. So we can
all work together to resolve this issue.

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Gary Thomas]

On 2011-10-07 04:08, Javier Martinez Canillas wrote:
> On Fri, Oct 7, 2011 at 11:34 AM, Gary Thomas<gary@mlbassoc.com>  wrote:
>> On 2011-10-06 10:11, Javier Martinez Canillas wrote:
>>>
>>> On Thu, Oct 6, 2011 at 5:47 PM, Gary Thomas<gary@mlbassoc.com>    wrote:
>>>>
>>>> On 2011-10-06 08:50, Javier Martinez Canillas wrote:
>>>>>
>>>>> On Thu, Oct 6, 2011 at 4:29 PM, Javier Martinez Canillas
>>>>> <martinez.javier@gmail.com>      wrote:
>>>>>>
>>>>>> On Thu, Oct 6, 2011 at 4:00 PM, Gary Thomas<gary@mlbassoc.com>
>>>>>>   wrote:
>>>>>>>
>>>>>>> On 2011-10-06 01:51, Javier Martinez Canillas wrote:
>>>>>>>>
>>>>>>>> On Wed, Oct 5, 2011 at 7:43 PM, Javier Martinez Canillas
>>>>>>>> <martinez.javier@gmail.com>        wrote:
>>>>>>>>>
>>>>>>>>> On Wed, Oct 5, 2011 at 6:28 PM, Enrico<ebutera@users.berlios.de>
>>>>>>>>>   wrote:
>>>>>>>>>>
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> since we are all interested in this driver (and tvp5150) i'll try
>>>>>>>>>> to
>>>>>>>>>> make a summary of the current situation and understand what is
>>>>>>>>>> needed
>>>>>>>>>> to finally get it into the main tree instead of having to apply a
>>>>>>>>>> dozen patches manually.
>>>>>>>>>>
>>>>>>>>>> The current status of git repositories/branches is:
>>>>>>>>>>
>>>>>>>>>> - main tree: working (i suppose) but no support for bt656 input
>>>>>>>>>>
>>>>>>>>>> - pinchartl/media:
>>>>>>>>>>   * omap3isp-omap3isp-next: i think it's in sync with linuxtv master
>>>>>>>>>> (for the omap3-isp parts)
>>>>>>>>>>   * omap3isp-omap3isp-yuv: like ..next but with some additional
>>>>>>>>>> format
>>>>>>>>>> patches
>>>>>>>>>>
>>>>>>>>>> "Floating" patches:
>>>>>>>>>>
>>>>>>>>>> - Deepthy: sent patches (against mainline) to add bt656 support
>>>>>>>>>>
>>>>>>>>>> Laurent made some comments, i haven't seen a v2 to be applied
>>>>>>>>>>
>>>>>>>>>> - Javier: sent patches for tvp5150, currently discussed on
>>>>>>>>>> linux-media; possible patches/fixes for omap3-isp
>>>>>>>>>>
>>>>>>>>
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> Since the patches are not against mainline I can't post for reviewing
>>>>>>>> but can be found in one of our development trees [1]. Comments are
>>>>>>>> highly appreciated.
>>>>>>>>
>>>>>>>> The tree is a 2.6.37 that already contain Deepthy patch. I rebased my
>>>>>>>> changes on top of that to correctly support both BT656 an non-BT656
>>>>>>>> video data processing.
>>>>>>>>
>>>>>>>> [1]:
>>>>>>>>
>>>>>>>>
>>>>>>>> http://git.igep.es/?p=pub/scm/linux-omap-2.6.git;a=shortlog;h=refs/heads/linux-2.6.37.y-next
>>>>>>>
>>>>>>> Any chance of rebasing these against a more up to date kernel, e.g.
>>>>>>> 3.2-working
>>>>>>> with the patches Laurent sent today?
>>>>>>>
>>>>>>
>>>>>> Sure, but I won't have time to do it neither today nor tomorrow. But
>>>>>> will do it during the weekend.
>>>>>>
>>>>>>>>>
>>>>>>>>> I will find some free time slots to resolve the issues called out by
>>>>>>>>> Sakari, Hans and Mauro and resend the patch-set for the tvp5151.
>>>>>>>>>
>>>>>>>>> Also I can send the patches of the modifications I made to the ISP
>>>>>>>>> driver. Right now I'm working on top of Deepthy patches.
>>>>>>>>>
>>>>>>>>> I can either send on top of that patch or rebase to mainline,
>>>>>>>>> whatever
>>>>>>>>> you think is better for reviewing.
>>>>>>>>>
>>>>>>>>>> Now what can we all do to converge to a final solution? I think
>>>>>>>>>> this
>>>>>>>>>> is also blocking the possible development/test of missing features,
>>>>>>>>>> like the recently-discussed resizer and cropping ones.
>>>>>>>>>>
>>>>>>>>>> Enrico
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Right now I have a working the tvp5151 with the ISP. I can capture
>>>>>>>>> ITU-R BT656 video both in PAL-M and NTSC standard. Also, the whole
>>>>>>>>> pipeline is configured automatically with the video standard
>>>>>>>>> detected
>>>>>>>>> by the tvp5151. Also, I'm using the CCDC to crop the frames and only
>>>>>>>>> capture the active lines for each standard (576 for PAL and 480 for
>>>>>>>>> NTSC) using the CCDC to crop the image.
>>>>>>>>>
>>>>>>>>
>>>>>>>> As I told you before video capturing is working for both PAL and NTSC
>>>>>>>> using standard V4L2 application (i.e: gstreamer) but the video still
>>>>>>>> shows some motion artifacts. Capturing YUV frames and looking at them
>>>>>>>> I realized that there does exist a pattern, the sequence 2 frames
>>>>>>>> correct and 3 frames with interlacing effects always repeats.
>>>>>>>
>>>>>>> I think I've seen this as well.  Could you provide a short video
>>>>>>> which shows the artefacts?
>>>>>>>
>>>>>>
>>>>>> Yes, I've attached a 16-frame video file. It is a PAL-M video
>>>>>> (720x576) in YUV 4:22 data format. Please let me know if it is OK for
>>>>>> you.
>>>>>>
>>>>>
>>>>> Sorry, I didn't notice the size of the image (13 MB) and got a lot of
>>>>> rejects from your MTAs. I uploaded the file to my personal github
>>>>> account [1].
>>>>>
>>>>> [1]:
>>>>> https://github.com/martinezjavier/omap3isp_tvp5151/blob/master/pal.yuv
>>>>
>>>> Very interesting.  What was your source (camera type, etc)?
>>>
>>> A samsung HD video camera connected to the TVP with its RCA video
>>> connector. But the artifact its independent of the analog input data,
>>> I've tried with an Sony Cybershot camera and other input sources.
>>>
>>>> How are you looking (or extracting) individual frames for analysis?
>>>>
>>>
>>> I'm using gstreamer to capture RAW YUV data and MPEG encoded video
>>> using the DSP.
>>>
>>> This are my pipelines:
>>>
>>> YUV:
>>>
>>> gst-launch-0.10 -v v4l2src device=/dev/video2 queue-size=16
>>> num-buffers=16 ! video/x-raw-yuv,format=\(fourcc\)UYVY ! filesink
>>> location=capture.out
>>>
>>> MPEG:
>>>
>>> gst-launch-0.10 -v v4l2src device=/dev/video2 queue-size=8 !
>>> video/x-raw-yuv,format=\(fourcc\)UYVY ! TIVidenc1 codecName=mpeg4enc
>>> engineName=codecServer ! qtmux ! filesink location=capture.m4v
>>>
>>>> I see much the same sort of artefacts as you are.  An example is at
>>>>   http://www.mlbassoc.com/misc/untitled.m2t
>>>> This is a little example I put together using kdenlive.  The first
>>>> segment
>>>> is the raw video from my camera, imported via USB.  The second is roughly
>>>> the same video captured using my OMAP board and converted to MP4 on the
>>>> fly
>>>> by this command:
>>>>   ffmpeg -r 30/1 -pix_fmt uyvy422 -s 720x524 -f video4linux2 -i
>>>> /dev/video2
>>>> -qscale 1 -f mp4 test1.mp4
>>>> I think there are some aspect ratio issues with these but what bothers me
>>>> the most is how much the captured data tears whenever there is a lot of
>>>> motion in the video.
>>
>> I figured out how to split your raw video into individual frames.  The
>> problems
>> don't look like only interlace issues to me.  Take a look at
>>   http://www.mlbassoc.com/misc/UYVY_examples/PAL_from_JavierMartinezCanillas/
>> especially image #9 which shows some very serious ghosting.
>>
>
> Yes, I don't know if this is because I'm not copying the sub-frame in
> the correct buffer or another.
>
>> I see the same behaviour here and it bothers me quite a lot.  I do hope that
>> we can figure out what's causing it - we have a number of customers that are
>> wanting to do analogue video capture using the OMAP+TVP5150, so it's pretty
>> important to us.
>>
>> Thanks for your time
>
> Yes, we are in the same situation. I did my best to fix this but I
> couldn't. I minimized the effect with those changes but there still
> exists.

Do we know for sure that these problems are happening in the ISP itself
or could they possibly be in the TVP5150?  Does anyone have experience
with a different analogue encoder?

>
> I hope as Enrico says that Laurent, Sakari (who im cc'ing here) or
> Deepthy that know better the ISP can enter the discussion. So we can
> all work together to resolve this issue.
>
> Best regards,
>

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Re: omap3-isp status

[Enrico]

On Fri, Oct 7, 2011 at 12:22 PM, Gary Thomas <gary@mlbassoc.com> wrote:
> Do we know for sure that these problems are happening in the ISP itself
> or could they possibly be in the TVP5150?  Does anyone have experience
> with a different analogue encoder?

Never tried another encoder, but at this point it's something to look
at. I don't think some TI people will say "yes the encoder has
ghosting artifacts".

Enrico

Re: omap3-isp status

[Javier Martinez Canillas]

On Fri, Oct 7, 2011 at 12:36 PM, Enrico <ebutera@users.berlios.de> wrote:
> On Fri, Oct 7, 2011 at 12:22 PM, Gary Thomas <gary@mlbassoc.com> wrote:
>> Do we know for sure that these problems are happening in the ISP itself
>> or could they possibly be in the TVP5150?  Does anyone have experience
>> with a different analogue encoder?
>
> Never tried another encoder, but at this point it's something to look
> at. I don't think some TI people will say "yes the encoder has
> ghosting artifacts".
>
> Enrico
>

I have never tried with an different decoder either. I don't think
this is a HW thing. As far as I know the tvp5150 is used in some
em28xx devices that is what Mauro said, and he would notice that
behaviour.

Also, if you try getting 625 lines (for PAL) but disable the
line-output-formatter for deinterlacing, i.e:

pdata->fldmode = 0;

ispccdc_config_outlineoffset(ccdc, pix.bytesperline, EVENEVEN, 0);
ispccdc_config_outlineoffset(ccdc, pix.bytesperline, EVENODD, 0);
ispccdc_config_outlineoffset(ccdc, pix.bytesperline, ODDEVEN, 0);
ispccdc_config_outlineoffset(ccdc, pix.bytesperline, ODDODD, 0);

Then you get a frame with the 313 odd lines and 312 even lines
correctly. That means that the TVP5151 is generating correctly the
interlaced video.

Also the ISP is doing correctly the deinterlacing for a some frames.
But all the approaches used so far (wait for two VD0 interrupt to
change the CCCDC output memory direction), looks more like a hack than
a clean solution to me, but maybe is the only way to do it with the
ISP.

My guess is that the problem is the ISP driver that before this
configuration (TVP5150/1 + ISP) had never been tested with an video
decoder that generates interlaced data.

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Gary Thomas]

On 2011-10-07 05:02, Javier Martinez Canillas wrote:
> On Fri, Oct 7, 2011 at 12:36 PM, Enrico<ebutera@users.berlios.de>  wrote:
>> On Fri, Oct 7, 2011 at 12:22 PM, Gary Thomas<gary@mlbassoc.com>  wrote:
>>> Do we know for sure that these problems are happening in the ISP itself
>>> or could they possibly be in the TVP5150?  Does anyone have experience
>>> with a different analogue encoder?
>>
>> Never tried another encoder, but at this point it's something to look
>> at. I don't think some TI people will say "yes the encoder has
>> ghosting artifacts".
>>
>> Enrico
>>
>
> I have never tried with an different decoder either. I don't think
> this is a HW thing. As far as I know the tvp5150 is used in some
> em28xx devices that is what Mauro said, and he would notice that
> behaviour.
>
> Also, if you try getting 625 lines (for PAL) but disable the
> line-output-formatter for deinterlacing, i.e:
>
> pdata->fldmode = 0;
>
> ispccdc_config_outlineoffset(ccdc, pix.bytesperline, EVENEVEN, 0);
> ispccdc_config_outlineoffset(ccdc, pix.bytesperline, EVENODD, 0);
> ispccdc_config_outlineoffset(ccdc, pix.bytesperline, ODDEVEN, 0);
> ispccdc_config_outlineoffset(ccdc, pix.bytesperline, ODDODD, 0);
>
> Then you get a frame with the 313 odd lines and 312 even lines
> correctly. That means that the TVP5151 is generating correctly the
> interlaced video.
>
> Also the ISP is doing correctly the deinterlacing for a some frames.
> But all the approaches used so far (wait for two VD0 interrupt to
> change the CCCDC output memory direction), looks more like a hack than
> a clean solution to me, but maybe is the only way to do it with the
> ISP.

Looking at your sequence of pictures, you can see that image #10 and #11
are pretty good, but #12..14 are all bad, then #15 & 16 are OK again.
In the bad ones, it looks like every other line has been shifted left by
some number of pixels.  It's hard to tell, but I think the shift is constant
when it happens.

>
> My guess is that the problem is the ISP driver that before this
> configuration (TVP5150/1 + ISP) had never been tested with an video
> decoder that generates interlaced data.

Of course, there's the comment in the manual that says it's not supported :-)
According to 12.4.4.1, BT656 (ITU) data can only use progressive scan sensors.

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Re: omap3-isp status

[Javier Martinez Canillas]

On Fri, Oct 7, 2011 at 1:39 PM, Gary Thomas <gary@mlbassoc.com> wrote:
> On 2011-10-07 05:02, Javier Martinez Canillas wrote:
>>
>> On Fri, Oct 7, 2011 at 12:36 PM, Enrico<ebutera@users.berlios.de>  wrote:
>>>
>>> On Fri, Oct 7, 2011 at 12:22 PM, Gary Thomas<gary@mlbassoc.com>  wrote:
>>>>
>>>> Do we know for sure that these problems are happening in the ISP itself
>>>> or could they possibly be in the TVP5150?  Does anyone have experience
>>>> with a different analogue encoder?
>>>
>>> Never tried another encoder, but at this point it's something to look
>>> at. I don't think some TI people will say "yes the encoder has
>>> ghosting artifacts".
>>>
>>> Enrico
>>>
>>
>> I have never tried with an different decoder either. I don't think
>> this is a HW thing. As far as I know the tvp5150 is used in some
>> em28xx devices that is what Mauro said, and he would notice that
>> behaviour.
>>
>> Also, if you try getting 625 lines (for PAL) but disable the
>> line-output-formatter for deinterlacing, i.e:
>>
>> pdata->fldmode = 0;
>>
>> ispccdc_config_outlineoffset(ccdc, pix.bytesperline, EVENEVEN, 0);
>> ispccdc_config_outlineoffset(ccdc, pix.bytesperline, EVENODD, 0);
>> ispccdc_config_outlineoffset(ccdc, pix.bytesperline, ODDEVEN, 0);
>> ispccdc_config_outlineoffset(ccdc, pix.bytesperline, ODDODD, 0);
>>
>> Then you get a frame with the 313 odd lines and 312 even lines
>> correctly. That means that the TVP5151 is generating correctly the
>> interlaced video.
>>
>> Also the ISP is doing correctly the deinterlacing for a some frames.
>> But all the approaches used so far (wait for two VD0 interrupt to
>> change the CCCDC output memory direction), looks more like a hack than
>> a clean solution to me, but maybe is the only way to do it with the
>> ISP.
>
> Looking at your sequence of pictures, you can see that image #10 and #11
> are pretty good, but #12..14 are all bad, then #15 & 16 are OK again.
> In the bad ones, it looks like every other line has been shifted left by
> some number of pixels.  It's hard to tell, but I think the shift is constant
> when it happens.
>

Yes the sequence is always the same 2 good, 3 bad. That is why I think
that is something related to buffers management.

>>
>> My guess is that the problem is the ISP driver that before this
>> configuration (TVP5150/1 + ISP) had never been tested with an video
>> decoder that generates interlaced data.
>
> Of course, there's the comment in the manual that says it's not supported
> :-)
> According to 12.4.4.1, BT656 (ITU) data can only use progressive scan
> sensors.
>
> --
> ------------------------------------------------------------
> Gary Thomas                 |  Consulting for the
> MLB Associates              |    Embedded world
> ------------------------------------------------------------
>



-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Laurent Pinchart]

Hi,

On Friday 07 October 2011 11:31:46 Javier Martinez Canillas wrote:
> On Fri, Oct 7, 2011 at 10:54 AM, Enrico wrote:
> > On Thu, Oct 6, 2011 at 6:05 PM, Javier Martinez Canillas wrote:
> >> On Thu, Oct 6, 2011 at 5:25 PM, Enrico wrote:
> >>> - i don't see Deepthy patches, it seems to be based on the
> >>> pre-Deepthy-patches driver and fixed (not that this is a bad thing!);
> >>> i say this because, like Gary, i'm interested in a possible forward
> >>> porting to a more recent kernel so i was searching for a starting
> >>> point
> >> 
> >> I didn't know there was a more recent version of Deepthy patches,
> >> Since they are not yet in mainline we should decide if we work on top
> >> of that or on top of mainline. Deepthy patches are very good to
> >> separate bt656 and non-bt656 execution inside the ISP, also add a
> >> platform data variable to decide which mode has to be used.
> >> 
> >> But reading the documentation and from my experimental validation I
> >> think that there are a few things that can be improved.
> >> 
> >> First the assumption that we can use FLDSTAT to check if a frame is
> >> ODD or EVEN I find to not always be true. Also I don't know who sets
> >> this value since in the TRM always talks as it is only used with
> >> discrete syncs.
> > 
> > Yes about FLDSTAT i noticed the same thing. And that's why we need
> > someone that knows the ISP better to help us....
> 
> Great, good to know that I'm not the only one that noticed this behavior.
> 
> >> Also, I don't think that we should change the ISP CCDC configuration
> >> inside the VD0 interrupt handler. Since the shadowed registers only
> >> can be accessed during a frame processing, or more formally the new
> >> values are taken at the beginning of a frame execution.
> >> 
> >> By the time we change for example the output address memory for the
> >> next buffer in the VD0 handler, the next frame is already being
> >> processed so that value won't be used for the CCDC until that frame
> >> finish. So It is not behaving as the code expect, since for 3 frames
> >> the CCDC output memory address will be the same.
> >> 
> >> That is why I move most of the logic to the VD1 interrupt since there
> >> the current frame didn't finish yet and we can configure the CCDC for
> >> the next frame.
> >> 
> >> But to do that the buffer for the next frame and the releasing of the
> >> last buffer can't happen simultaneously, that is why I decouple these
> >> two actions.
> >> 
> >> Again, this is my own observations and what I understood from the TRM
> >> and I could be wrong.
> > 
> > I can't comment on that, i hope Laurent or Deepthy will join the
> > discussion...
> 
> I second you on that, we need someone who knows the ISP better than we
> do. I have to fix this anyway, so it is better if I can do it the
> right way and the code gos upstream, so we don't have to internally
> maintain a separate patch-set and forward port for each kernel release
> we do.

Two quick comments, as I haven't had time to look into this recently.

1. I've updated the omap3isp-omap3isp-yuv branch with a new CCDC YUV support 
patch which should (hopefully) configure the bridge automatically and report 
correct formats at the CCDC output. The patch hasn't been tested as I still 
don't have access to YUV hardware.

2. Could you guys please rebase all your patches on top of the omap3isp-
omap3isp-yuv branch ? I will then review them.

> >>> - i don't think that adding the "priv" field in v4l2-mediabus.h will
> >>> be accepted, and since it is related to the default cropping you added
> >>> i think it can be dropped and just let the user choose the appropriate
> >>> cropping
> >> 
> >> Yes, probably is too much of a hack, but I didn't know of another way
> >> that the subdev could report to the ISP of the standard and since
> >> v4l2_pix_format has also a priv field, I think it could be at least a
> >> temporary solution (remember that we want this to work first and then
> >> we plan to do it right for upstream submission).
> > 
> > ...and my hope continues here.

-- 
Regards,

Laurent Pinchart

Re: omap3-isp status

[Javier Martinez Canillas]

On Sat, Oct 8, 2011 at 5:51 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi,
>
> On Friday 07 October 2011 11:31:46 Javier Martinez Canillas wrote:
>> On Fri, Oct 7, 2011 at 10:54 AM, Enrico wrote:
>> > On Thu, Oct 6, 2011 at 6:05 PM, Javier Martinez Canillas wrote:
>> >> On Thu, Oct 6, 2011 at 5:25 PM, Enrico wrote:
>> >>> - i don't see Deepthy patches, it seems to be based on the
>> >>> pre-Deepthy-patches driver and fixed (not that this is a bad thing!);
>> >>> i say this because, like Gary, i'm interested in a possible forward
>> >>> porting to a more recent kernel so i was searching for a starting
>> >>> point
>> >>
>> >> I didn't know there was a more recent version of Deepthy patches,
>> >> Since they are not yet in mainline we should decide if we work on top
>> >> of that or on top of mainline. Deepthy patches are very good to
>> >> separate bt656 and non-bt656 execution inside the ISP, also add a
>> >> platform data variable to decide which mode has to be used.
>> >>
>> >> But reading the documentation and from my experimental validation I
>> >> think that there are a few things that can be improved.
>> >>
>> >> First the assumption that we can use FLDSTAT to check if a frame is
>> >> ODD or EVEN I find to not always be true. Also I don't know who sets
>> >> this value since in the TRM always talks as it is only used with
>> >> discrete syncs.
>> >
>> > Yes about FLDSTAT i noticed the same thing. And that's why we need
>> > someone that knows the ISP better to help us....
>>
>> Great, good to know that I'm not the only one that noticed this behavior.
>>
>> >> Also, I don't think that we should change the ISP CCDC configuration
>> >> inside the VD0 interrupt handler. Since the shadowed registers only
>> >> can be accessed during a frame processing, or more formally the new
>> >> values are taken at the beginning of a frame execution.
>> >>
>> >> By the time we change for example the output address memory for the
>> >> next buffer in the VD0 handler, the next frame is already being
>> >> processed so that value won't be used for the CCDC until that frame
>> >> finish. So It is not behaving as the code expect, since for 3 frames
>> >> the CCDC output memory address will be the same.
>> >>
>> >> That is why I move most of the logic to the VD1 interrupt since there
>> >> the current frame didn't finish yet and we can configure the CCDC for
>> >> the next frame.
>> >>
>> >> But to do that the buffer for the next frame and the releasing of the
>> >> last buffer can't happen simultaneously, that is why I decouple these
>> >> two actions.
>> >>
>> >> Again, this is my own observations and what I understood from the TRM
>> >> and I could be wrong.
>> >
>> > I can't comment on that, i hope Laurent or Deepthy will join the
>> > discussion...
>>
>> I second you on that, we need someone who knows the ISP better than we
>> do. I have to fix this anyway, so it is better if I can do it the
>> right way and the code gos upstream, so we don't have to internally
>> maintain a separate patch-set and forward port for each kernel release
>> we do.
>
> Two quick comments, as I haven't had time to look into this recently.
>
> 1. I've updated the omap3isp-omap3isp-yuv branch with a new CCDC YUV support
> patch which should (hopefully) configure the bridge automatically and report
> correct formats at the CCDC output. The patch hasn't been tested as I still
> don't have access to YUV hardware.
>

Hello Laurent, I'm glad to see that you are joining the thread :)

> 2. Could you guys please rebase all your patches on top of the omap3isp-
> omap3isp-yuv branch ? I will then review them.
>

Yes, I'll cook a patch today on top on your omap3isp-yuv and send for
review. I won't be able to test neither since I don't have proper
hardware at home. But at least you will get an idea of the approach we
are using to solve this and can point possible flaws.

>> >>> - i don't think that adding the "priv" field in v4l2-mediabus.h will
>> >>> be accepted, and since it is related to the default cropping you added
>> >>> i think it can be dropped and just let the user choose the appropriate
>> >>> cropping
>> >>
>> >> Yes, probably is too much of a hack, but I didn't know of another way
>> >> that the subdev could report to the ISP of the standard and since
>> >> v4l2_pix_format has also a priv field, I think it could be at least a
>> >> temporary solution (remember that we want this to work first and then
>> >> we plan to do it right for upstream submission).
>> >
>> > ...and my hope continues here.
>
> --
> Regards,
>
> Laurent Pinchart
>

Thanks a lot for your time.

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Enrico]

On Sat, Oct 8, 2011 at 6:11 PM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> Yes, I'll cook a patch today on top on your omap3isp-yuv and send for
> review. I won't be able to test neither since I don't have proper
> hardware at home. But at least you will get an idea of the approach we
> are using to solve this and can point possible flaws.

I made some tests and unfortunately there are some problems.

Note: i made these tests picking some patches from omap3isp-yuv branch
and applying to igep 3.1.0rc9 kernel (more or less like mainline +
some bsp patches) so maybe i made some mistakes (the tvp5150 driver is
patched too), but due to the nature of the problems i don't think this
is the case.

Javier patches v1: i can grab frames with yavta but i get only garbage.

Javier patches v2: i cannot grab frames with yavta (it hangs). I see
only 2 interrupts on the isp, then stops.

I compared Javier-v2 registers setup with Deepthy's patches and there
are some differences. Moreover i remember that in Deepthy patches vd1
interrupt was not used (and in fact i had the same yavta-hanging
problem before, and Deepthy patches solved it).

I think Javier-v1 patches didn't hang the isp because they changed
vd0/vd1 logic too, so maybe there were only some wrong isp registers
but the logic was correct.

Now i wonder if it could be easier/better to port Deepthy patches
first and then add Javier fixes...

Enrico

Re: omap3-isp status

[Javier Martinez Canillas]

On Mon, Oct 10, 2011 at 12:35 AM, Enrico <ebutera@users.berlios.de> wrote:
> On Sat, Oct 8, 2011 at 6:11 PM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> Yes, I'll cook a patch today on top on your omap3isp-yuv and send for
>> review. I won't be able to test neither since I don't have proper
>> hardware at home. But at least you will get an idea of the approach we
>> are using to solve this and can point possible flaws.
>
> I made some tests and unfortunately there are some problems.
>
> Note: i made these tests picking some patches from omap3isp-yuv branch
> and applying to igep 3.1.0rc9 kernel (more or less like mainline +
> some bsp patches) so maybe i made some mistakes (the tvp5150 driver is
> patched too), but due to the nature of the problems i don't think this
> is the case.
>
> Javier patches v1: i can grab frames with yavta but i get only garbage.
>
> Javier patches v2: i cannot grab frames with yavta (it hangs). I see
> only 2 interrupts on the isp, then stops.
>
> I compared Javier-v2 registers setup with Deepthy's patches and there
> are some differences. Moreover i remember that in Deepthy patches vd1
> interrupt was not used (and in fact i had the same yavta-hanging
> problem before, and Deepthy patches solved it).
>
> I think Javier-v1 patches didn't hang the isp because they changed
> vd0/vd1 logic too, so maybe there were only some wrong isp registers
> but the logic was correct.
>
> Now i wonder if it could be easier/better to port Deepthy patches
> first and then add Javier fixes...
>
> Enrico
>

Hi Enrico,

Yes, you are right in interlaced mode the VD1 interrupt handler
doesn't have to be executed. In v1 there is a conditional execution
and that is why the ISP doesn't hang up.

Could you please try changing this on ispccdc.c with v2 patches?

diff --git a/drivers/media/video/omap3isp/ispccdc.c
b/drivers/media/video/omap3isp/ispccdc.c
index 9b40968..bfd3f46 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -1658,7 +1658,8 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device
*ccdc, u32 events)
        if (ccdc->state == ISP_PIPELINE_STREAM_STOPPED)
                return 0;

-       if (events & IRQ0STATUS_CCDC_VD1_IRQ)
+       if ((events & IRQ0STATUS_CCDC_VD1_IRQ) &&
+           !ccdc_input_is_fldmode(ccdc))
                ccdc_vd1_isr(ccdc);

        ccdc_lsc_isr(ccdc, events);

With this change the ISP shouldn't hang but I don't know if you will
get the right data.

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Enrico]

On Mon, Oct 10, 2011 at 1:00 AM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Mon, Oct 10, 2011 at 12:35 AM, Enrico <ebutera@users.berlios.de> wrote:
>> I made some tests and unfortunately there are some problems.
>>
>> Note: i made these tests picking some patches from omap3isp-yuv branch
>> and applying to igep 3.1.0rc9 kernel (more or less like mainline +
>> some bsp patches) so maybe i made some mistakes (the tvp5150 driver is
>> patched too), but due to the nature of the problems i don't think this
>> is the case.
>>
>> Javier patches v1: i can grab frames with yavta but i get only garbage.
>>
>> Javier patches v2: i cannot grab frames with yavta (it hangs). I see
>> only 2 interrupts on the isp, then stops.
>>
>> I compared Javier-v2 registers setup with Deepthy's patches and there
>> are some differences. Moreover i remember that in Deepthy patches vd1
>> interrupt was not used (and in fact i had the same yavta-hanging
>> problem before, and Deepthy patches solved it).
>>
>> I think Javier-v1 patches didn't hang the isp because they changed
>> vd0/vd1 logic too, so maybe there were only some wrong isp registers
>> but the logic was correct.
>>
>> Now i wonder if it could be easier/better to port Deepthy patches
>> first and then add Javier fixes...
>>
>> Enrico
>>
>
> Hi Enrico,
>
> Yes, you are right in interlaced mode the VD1 interrupt handler
> doesn't have to be executed. In v1 there is a conditional execution
> and that is why the ISP doesn't hang up.
>
> Could you please try changing this on ispccdc.c with v2 patches?
>
> diff --git a/drivers/media/video/omap3isp/ispccdc.c
> b/drivers/media/video/omap3isp/ispccdc.c
> index 9b40968..bfd3f46 100644
> --- a/drivers/media/video/omap3isp/ispccdc.c
> +++ b/drivers/media/video/omap3isp/ispccdc.c
> @@ -1658,7 +1658,8 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device
> *ccdc, u32 events)
>        if (ccdc->state == ISP_PIPELINE_STREAM_STOPPED)
>                return 0;
>
> -       if (events & IRQ0STATUS_CCDC_VD1_IRQ)
> +       if ((events & IRQ0STATUS_CCDC_VD1_IRQ) &&
> +           !ccdc_input_is_fldmode(ccdc))
>                ccdc_vd1_isr(ccdc);
>
>        ccdc_lsc_isr(ccdc, events);
>
> With this change the ISP shouldn't hang but I don't know if you will
> get the right data.

I tried that but i only get this:

root@igep0020:~# yavta -c4 -f UYVY -s 720x625 -n 4 /dev/video2
Device /dev/video2 opened.
Device `OMAP3 ISP CCDC output' on `media' is a video capture device.
Video format set: UYVY (59565955) 720x625 buffer size 900000
Video format: UYVY (59565955) 720x625 buffer size 900000
4 buffers requested.
length: 900000 offset: 0
Buffer 0 mapped at address 0x4027a000.
length: 900000 offset: 901120
Buffer 1 mapped at address 0x403de000.
length: 900000 offset: 1802240
Buffer 2 mapped at address 0x4059b000.
length: 900000 offset: 2703360
Buffer 3 mapped at address 0x40748000.
[  952.675170] omap3isp omap3isp: CCDC won't become idle!
[  952.695159] omap3isp omap3isp: CCDC won't become idle!
[  952.715179] omap3isp omap3isp: CCDC won't become idle!
[  952.735137] omap3isp omap3isp: CCDC won't become idle!

and it continues forever saying that.

I'm going to try to apply Deepthy patches on omap3isp-yuv and hope it will work.

Enrico

Re: omap3-isp status

[Javier Martinez Canillas]

On Mon, Oct 10, 2011 at 10:54 AM, Enrico <ebutera@users.berlios.de> wrote:
> On Mon, Oct 10, 2011 at 1:00 AM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> On Mon, Oct 10, 2011 at 12:35 AM, Enrico <ebutera@users.berlios.de> wrote:
>>> I made some tests and unfortunately there are some problems.
>>>
>>> Note: i made these tests picking some patches from omap3isp-yuv branch
>>> and applying to igep 3.1.0rc9 kernel (more or less like mainline +
>>> some bsp patches) so maybe i made some mistakes (the tvp5150 driver is
>>> patched too), but due to the nature of the problems i don't think this
>>> is the case.
>>>
>>> Javier patches v1: i can grab frames with yavta but i get only garbage.
>>>
>>> Javier patches v2: i cannot grab frames with yavta (it hangs). I see
>>> only 2 interrupts on the isp, then stops.
>>>
>>> I compared Javier-v2 registers setup with Deepthy's patches and there
>>> are some differences. Moreover i remember that in Deepthy patches vd1
>>> interrupt was not used (and in fact i had the same yavta-hanging
>>> problem before, and Deepthy patches solved it).
>>>
>>> I think Javier-v1 patches didn't hang the isp because they changed
>>> vd0/vd1 logic too, so maybe there were only some wrong isp registers
>>> but the logic was correct.
>>>
>>> Now i wonder if it could be easier/better to port Deepthy patches
>>> first and then add Javier fixes...
>>>
>>> Enrico
>>>
>>
>> Hi Enrico,
>>
>> Yes, you are right in interlaced mode the VD1 interrupt handler
>> doesn't have to be executed. In v1 there is a conditional execution
>> and that is why the ISP doesn't hang up.
>>
>> Could you please try changing this on ispccdc.c with v2 patches?
>>
>> diff --git a/drivers/media/video/omap3isp/ispccdc.c
>> b/drivers/media/video/omap3isp/ispccdc.c
>> index 9b40968..bfd3f46 100644
>> --- a/drivers/media/video/omap3isp/ispccdc.c
>> +++ b/drivers/media/video/omap3isp/ispccdc.c
>> @@ -1658,7 +1658,8 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device
>> *ccdc, u32 events)
>>        if (ccdc->state == ISP_PIPELINE_STREAM_STOPPED)
>>                return 0;
>>
>> -       if (events & IRQ0STATUS_CCDC_VD1_IRQ)
>> +       if ((events & IRQ0STATUS_CCDC_VD1_IRQ) &&
>> +           !ccdc_input_is_fldmode(ccdc))
>>                ccdc_vd1_isr(ccdc);
>>
>>        ccdc_lsc_isr(ccdc, events);
>>
>> With this change the ISP shouldn't hang but I don't know if you will
>> get the right data.
>
> I tried that but i only get this:
>
> root@igep0020:~# yavta -c4 -f UYVY -s 720x625 -n 4 /dev/video2
> Device /dev/video2 opened.
> Device `OMAP3 ISP CCDC output' on `media' is a video capture device.
> Video format set: UYVY (59565955) 720x625 buffer size 900000
> Video format: UYVY (59565955) 720x625 buffer size 900000
> 4 buffers requested.
> length: 900000 offset: 0
> Buffer 0 mapped at address 0x4027a000.
> length: 900000 offset: 901120
> Buffer 1 mapped at address 0x403de000.
> length: 900000 offset: 1802240
> Buffer 2 mapped at address 0x4059b000.
> length: 900000 offset: 2703360
> Buffer 3 mapped at address 0x40748000.
> [  952.675170] omap3isp omap3isp: CCDC won't become idle!
> [  952.695159] omap3isp omap3isp: CCDC won't become idle!
> [  952.715179] omap3isp omap3isp: CCDC won't become idle!
> [  952.735137] omap3isp omap3isp: CCDC won't become idle!
>
> and it continues forever saying that.
>
> I'm going to try to apply Deepthy patches on omap3isp-yuv and hope it will work.
>
> Enrico
>

Hi Enrico,

Is your tree (igep 3.1.0rc9 kernel + omap3isp-yuv patches) accessible
so I can clone and give a try?

I can then make a patch-set on top on that, one that I can actually
test on real hardware and be sure that is working well.

If I can git clone your tree then it will be faster, otherwise I will
try omap3isp-yuv with igep board support added, but it would take me
more time to do so. I will find some free time late this afternoon to
try that.

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Enrico]

On Mon, Oct 10, 2011 at 11:02 AM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> Is your tree (igep 3.1.0rc9 kernel + omap3isp-yuv patches) accessible
> so I can clone and give a try?
>
> I can then make a patch-set on top on that, one that I can actually
> test on real hardware and be sure that is working well.
>
> If I can git clone your tree then it will be faster, otherwise I will
> try omap3isp-yuv with igep board support added, but it would take me
> more time to do so. I will find some free time late this afternoon to
> try that.

I have updated my igep openembedded layer at [1] (testing branch) with:

current igep master kernel (3.1.0rc9) + omap3isp-yuv patches + your
patches v2 + tvp5150 patches + some tvp5150 and board files fixes.

All the patches (specified in the .bb file) are git am-able patches so
you can just clone the igep master repository and apply all the
patches (0001-0025).

This is the cover letter for the patches i applied, if someone can
review the omap3isp related patches to be sure i didn't forget
anything it would be very helpful:

Arnaud Lacombe (1):
  drivers/media: do not use EXTRA_CFLAGS

Enrico Butera (3):
  tvp5150: compile fixes and added missing entity_cleanup
  exp-igep0022: add tvp5151 support
  igep00x0: fix camera platform data

Guennadi Liakhovetski (1):
  omap3isp: ccdc: remove redundant operation

Ivaylo Petrov (1):
  omap3isp: csi2: Add V4L2_MBUS_FMT_YUYV8_2X8 support

Javier Martinez Canillas (6):
  omap3isp: ccdc: Add interlaced field mode to platform data
  omap3isp: ccdc: Add interlaced count field to isp_ccdc_device
  omap3isp: ccdc: Add support to ITU-R BT.656 video data format
  tvp5150: Add constants for PAL and NTSC video standards
  tvp5150: Add video format registers configuration values
  tvp5150: Migrate to media-controller framework and add video format
    detection

Laurent Pinchart (12):
  omap3isp: Don't accept pipelines with no video source as valid
  omap3isp: Move platform data definitions from isp.h to
    media/omap3isp.h
  omap3isp: Don't fail streamon when the sensor doesn't implement
    s_stream
  omap3isp: video: Avoid crashes when pipeline set stream operation
    fails
  omap3isp: Move media_entity_cleanup() from unregister() to cleanup()
  omap3isp: Move *_init_entities() functions to the init/cleanup
    section
  omap3isp: Add missing mutex_destroy() calls
  omap3isp: Fix memory leaks in initialization error paths
  omap3isp: video: Split format info bpp field into width and bpp
  omap3isp: ccdc: Remove support for interlaced data and master HS/VS
    mode
  omap3isp: ccdc: Remove ispccdc_syncif structure
  omap3isp: ccdc: Add YUV input formats support

Michael Jones (1):
  omap3isp: queue: fail QBUF if user buffer is too small

 arch/arm/mach-omap2/board-igep00x0.c       |   67 +++++
 arch/arm/mach-omap2/exp-igep0022.c         |    3 +
 drivers/media/video/omap3isp/Makefile      |    4 +-
 drivers/media/video/omap3isp/isp.c         |   13 +-
 drivers/media/video/omap3isp/isp.h         |   87 +------
 drivers/media/video/omap3isp/ispccdc.c     |  376 ++++++++++++++++----------
 drivers/media/video/omap3isp/ispccdc.h     |   38 +---
 drivers/media/video/omap3isp/ispccp2.c     |  129 +++++-----
 drivers/media/video/omap3isp/ispcsi2.c     |  118 +++++---
 drivers/media/video/omap3isp/isph3a_aewb.c |    2 +-
 drivers/media/video/omap3isp/isph3a_af.c   |    2 +-
 drivers/media/video/omap3isp/isphist.c     |    2 +-
 drivers/media/video/omap3isp/isppreview.c  |  108 ++++----
 drivers/media/video/omap3isp/ispqueue.c    |    4 +
 drivers/media/video/omap3isp/ispresizer.c  |  104 ++++----
 drivers/media/video/omap3isp/ispstat.c     |   52 ++--
 drivers/media/video/omap3isp/ispstat.h     |    2 +-
 drivers/media/video/omap3isp/ispvideo.c    |   77 ++++--
 drivers/media/video/omap3isp/ispvideo.h    |    5 +-
 drivers/media/video/tvp5150.c              |  408 +++++++++++++++++++++++++++-
 drivers/media/video/tvp5150_reg.h          |   17 +-
 include/media/omap3isp.h                   |  138 ++++++++++
 include/media/tvp5150.h                    |    6 +
 23 files changed, 1215 insertions(+), 547 deletions(-)
 create mode 100644 include/media/omap3isp.h

Enrico

Re: omap3-isp status

[Enrico]

On Mon, Oct 10, 2011 at 12:06 PM, Enrico <ebutera@users.berlios.de> wrote:
> I have updated my igep openembedded layer at [1] (testing branch) with:

Ops, forgot [1] !

[1]: https://github.com/ebutera/meta-igep

Enrico

Re: omap3-isp status

[Javier Martinez Canillas]

On Mon, Oct 10, 2011 at 12:07 PM, Enrico <ebutera@users.berlios.de> wrote:
> On Mon, Oct 10, 2011 at 12:06 PM, Enrico <ebutera@users.berlios.de> wrote:
>> I have updated my igep openembedded layer at [1] (testing branch) with:
>
> Ops, forgot [1] !
>
> [1]: https://github.com/ebutera/meta-igep
>
> Enrico
>

Perfect, thank you Enrico. I will try this latter today and let you
know. I'm sure I can get this working (with the ghosting effect of
course) so you can at least obtain 25 fps and once I have this working
I will resend the patch-set as v3 so Laurent can review it and
hopefully help us to fix the artifact on the video.

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Enrico]

[-- Attachment #1: Type: text/plain, Size: 1242 bytes --]

On Mon, Oct 10, 2011 at 12:33 PM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Mon, Oct 10, 2011 at 12:07 PM, Enrico <ebutera@users.berlios.de> wrote:
>> On Mon, Oct 10, 2011 at 12:06 PM, Enrico <ebutera@users.berlios.de> wrote:
>>> I have updated my igep openembedded layer at [1] (testing branch) with:
>>
>> Ops, forgot [1] !
>>
>> [1]: https://github.com/ebutera/meta-igep
>>
>> Enrico
>>
>
> Perfect, thank you Enrico. I will try this latter today and let you
> know. I'm sure I can get this working (with the ghosting effect of
> course) so you can at least obtain 25 fps and once I have this working
> I will resend the patch-set as v3 so Laurent can review it and
> hopefully help us to fix the artifact on the video.

Yes it must be something simple but not easy to spot.

And to add even more confusion i've attached two patches: one is a
port of two Deepthy patches, the other one just a board fix.

Just replace patches 0017-18-19 with the attached 0001 patch, and
after patch 0025 apply the attached 0002 patch.

With these i can succesfully grab frames with yavta BUT i only get
half-height frames. Disclaimer: i just made the patch monkey and gave
it a run without a review so it could be anything.

Enrico

[-- Attachment #2: 0001-omap3isp-configure-ccdc-and-add-bt656-support.patch --]
[-- Type: text/x-patch, Size: 11807 bytes --]

From abb4d9df937ebda4f628d5b94ae02c60e7b06a7b Mon Sep 17 00:00:00 2001
From: Enrico Butera <ebutera@users.berlios.de>
Date: Mon, 10 Oct 2011 14:16:46 +0200
Subject: [PATCH] omap3isp: configure ccdc and add bt656 support

This is a port of the following Deepthy Ravi patches:

[PATCH 5/8] ispccdc: Configure CCDC registers
[PATCH 6/8] ispccdc: Add support for BT656 interface

Signed-off-by: Enrico Butera <ebutera@users.berlios.de>
---
 drivers/media/video/omap3isp/ispccdc.c  |  126 +++++++++++++++++++++++++------
 drivers/media/video/omap3isp/ispccdc.h  |    1 +
 drivers/media/video/omap3isp/ispreg.h   |    1 +
 drivers/media/video/omap3isp/ispvideo.c |    2 +-
 drivers/media/video/omap3isp/ispvideo.h |    4 +-
 5 files changed, 110 insertions(+), 24 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index ce0e57d..3903acf 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -60,8 +60,11 @@ static const unsigned int ccdc_fmts[] = {
 	V4L2_MBUS_FMT_SGBRG12_1X12,
 	V4L2_MBUS_FMT_YUYV8_2X8,
 	V4L2_MBUS_FMT_UYVY8_2X8,
+	V4L2_MBUS_FMT_YUYV8_2X8,
 };
 
+static bool ccdc_input_is_bt656(struct isp_ccdc_device *ccdc);
+
 /*
  * ccdc_print_status - Print current CCDC Module register values.
  * @ccdc: Pointer to ISP CCDC device.
@@ -793,11 +796,16 @@ static void ccdc_apply_controls(struct isp_ccdc_device *ccdc)
 void omap3isp_ccdc_restore_context(struct isp_device *isp)
 {
 	struct isp_ccdc_device *ccdc = &isp->isp_ccdc;
+	struct v4l2_mbus_framefmt *format;
 
 	isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG, ISPCCDC_CFG_VDLC);
 
-	ccdc->update = OMAP3ISP_CCDC_ALAW | OMAP3ISP_CCDC_LPF
-		     | OMAP3ISP_CCDC_BLCLAMP | OMAP3ISP_CCDC_BCOMP;
+	/* CCDC_PAD_SINK */
+	format = &ccdc->formats[CCDC_PAD_SINK];
+	if ((format->code != V4L2_MBUS_FMT_UYVY8_2X8) &&
+			(format->code != V4L2_MBUS_FMT_UYVY8_2X8))
+		ccdc->update = OMAP3ISP_CCDC_ALAW | OMAP3ISP_CCDC_LPF
+				| OMAP3ISP_CCDC_BLCLAMP | OMAP3ISP_CCDC_BCOMP;
 	ccdc_apply_controls(ccdc);
 	ccdc_configure_fpc(ccdc);
 }
@@ -1022,10 +1030,10 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 
 	if (pdata && pdata->bt656)
 		isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF,
-			    ISPCCDC_REC656IF_R656ON);
+			    ISPCCDC_REC656IF_R656ON | ISPCCDC_REC656IF_ECCFVH);
 	else
 		isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF,
-			    ISPCCDC_REC656IF_R656ON);
+			    ISPCCDC_REC656IF_R656ON | ISPCCDC_REC656IF_ECCFVH);
 }
 
 /* CCDC formats descriptions */
@@ -1107,6 +1115,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	struct isp_parallel_platform_data *pdata = NULL;
 	struct v4l2_subdev *sensor;
 	struct v4l2_mbus_framefmt *format;
+	struct v4l2_pix_format pix;
 	const struct isp_format_info *fmt_info;
 	struct v4l2_subdev_format fmt_src;
 	unsigned int depth_out;
@@ -1165,6 +1174,9 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	/* CCDC_PAD_SINK */
 	format = &ccdc->formats[CCDC_PAD_SINK];
 
+	if (format->code == V4L2_MBUS_FMT_UYVY8_2X8)
+		isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG,
+			    ISPCCDC_CFG_Y8POS);
 	/* Mosaic filter */
 	switch (format->code) {
 	case V4L2_MBUS_FMT_SRGGB10_1X10:
@@ -1184,27 +1196,59 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 		ccdc_pattern = ccdc_sgrbg_pattern;
 		break;
 	}
-	ccdc_config_imgattr(ccdc, ccdc_pattern);
 
-	/* Generate VD0 on the last line of the image and VD1 on the
-	 * 2/3 height line.
-	 */
-	isp_reg_writel(isp, ((format->height - 2) << ISPCCDC_VDINT_0_SHIFT) |
-		       ((format->height * 2 / 3) << ISPCCDC_VDINT_1_SHIFT),
-		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VDINT);
+	if ((format->code != V4L2_MBUS_FMT_YUYV8_2X8) &&
+			(format->code != V4L2_MBUS_FMT_UYVY8_2X8))
+		ccdc_config_imgattr(ccdc, ccdc_pattern);
+
+	/* BT656: Generate VD0 on the last line of each field, and we
+	* don't use VD1.
+	* Non BT656: Generate VD0 on the last line of the image and VD1 on the
+	* 2/3 height line.
+	*/
+	if (pdata->bt656)
+		isp_reg_writel(isp,
+			(format->height/2 - 2) << ISPCCDC_VDINT_0_SHIFT,
+			OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VDINT);
+	else
+		isp_reg_writel(isp,
+			((format->height - 2) << ISPCCDC_VDINT_0_SHIFT) |
+			((format->height * 2 / 3) << ISPCCDC_VDINT_1_SHIFT),
+			OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VDINT);
 
 	/* CCDC_PAD_SOURCE_OF */
 	format = &ccdc->formats[CCDC_PAD_SOURCE_OF];
 
-	isp_reg_writel(isp, (0 << ISPCCDC_HORZ_INFO_SPH_SHIFT) |
+	/* For BT656 the number of bytes would be width*2 */
+	if (pdata->bt656)
+		isp_reg_writel(isp, (0 << ISPCCDC_HORZ_INFO_SPH_SHIFT) |
+			((format->width * 2 - 1) << ISPCCDC_HORZ_INFO_NPH_SHIFT),
+			OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HORZ_INFO);
+	else
+		isp_reg_writel(isp, (0 << ISPCCDC_HORZ_INFO_SPH_SHIFT) |
 		       ((format->width - 1) << ISPCCDC_HORZ_INFO_NPH_SHIFT),
 		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HORZ_INFO);
 	isp_reg_writel(isp, 0 << ISPCCDC_VERT_START_SLV0_SHIFT,
 		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_START);
-	isp_reg_writel(isp, (format->height - 1)
+
+	if (pdata->bt656)
+		isp_reg_writel(isp, ((format->height >> 1) - 1)
+			<< ISPCCDC_VERT_LINES_NLV_SHIFT,
+			OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_LINES);
+	else
+		isp_reg_writel(isp, (format->height - 1)
 			<< ISPCCDC_VERT_LINES_NLV_SHIFT,
 		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_LINES);
 
+	isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
+		    ISPCCDC_SDOFST_LOFST_MASK << ISPCCDC_SDOFST_LOFST0_SHIFT);
+	isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
+		    ISPCCDC_SDOFST_LOFST_MASK << ISPCCDC_SDOFST_LOFST1_SHIFT);
+	isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
+		    ISPCCDC_SDOFST_LOFST_MASK << ISPCCDC_SDOFST_LOFST2_SHIFT);
+	isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
+		    ISPCCDC_SDOFST_LOFST_MASK << ISPCCDC_SDOFST_LOFST3_SHIFT);
+
 	ccdc_config_outlineoffset(ccdc, ccdc->video_out.bpl_value, 0, 0);
 
 	/* CCDC_PAD_SOURCE_VP */
@@ -1224,8 +1268,14 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	/* Use PACK8 mode for 1byte per pixel formats. */
 	if (omap3isp_video_format_info(format->code)->width <= 8)
 		syn_mode |= ISPCCDC_SYN_MODE_PACK8;
-	else
-		syn_mode &= ~ISPCCDC_SYN_MODE_PACK8;
+
+	if ((format->code == V4L2_MBUS_FMT_YUYV8_2X8) ||
+		(format->code == V4L2_MBUS_FMT_UYVY8_2X8)) {
+		if (pdata->bt656)
+			syn_mode |= ISPCCDC_SYN_MODE_INPMOD_YCBCR8;
+		else
+			syn_mode |= ISPCCDC_SYN_MODE_INPMOD_YCBCR16;
+	}
 
 	isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
 
@@ -1252,6 +1302,11 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 unlock:
 	spin_unlock_irqrestore(&ccdc->lsc.req_lock, flags);
 
+	if (pdata->bt656)
+		ccdc->update = OMAP3ISP_CCDC_BLCLAMP;
+	else
+		ccdc->update = 0;
+
 	ccdc_apply_controls(ccdc);
 }
 
@@ -1263,6 +1318,7 @@ static void __ccdc_enable(struct isp_ccdc_device *ccdc, int enable)
 			ISPCCDC_PCR_EN, enable ? ISPCCDC_PCR_EN : 0);
 }
 
+static int __ccdc_handle_stopping(struct isp_ccdc_device *ccdc, u32 event);
 static int ccdc_disable(struct isp_ccdc_device *ccdc)
 {
 	unsigned long flags;
@@ -1273,6 +1329,11 @@ static int ccdc_disable(struct isp_ccdc_device *ccdc)
 		ccdc->stopping = CCDC_STOP_REQUEST;
 	spin_unlock_irqrestore(&ccdc->lock, flags);
 
+	__ccdc_lsc_enable(ccdc, 0);
+	__ccdc_enable(ccdc, 0);
+	ccdc->stopping = CCDC_STOP_EXECUTED;
+	__ccdc_handle_stopping(ccdc, CCDC_STOP_FINISHED);
+
 	ret = wait_event_timeout(ccdc->wait,
 				 ccdc->stopping == CCDC_STOP_FINISHED,
 				 msecs_to_jiffies(2000));
@@ -1521,10 +1582,29 @@ static void ccdc_vd0_isr(struct isp_ccdc_device *ccdc)
 {
 	unsigned long flags;
 	int restart = 0;
+	struct isp_device *isp = to_isp_device(ccdc);
 
-	if (ccdc->output & CCDC_OUTPUT_MEMORY)
-		restart = ccdc_isr_buffer(ccdc);
-
+	if (ccdc->output & CCDC_OUTPUT_MEMORY) {
+		if (ccdc_input_is_bt656(ccdc)) {
+			u32 fid;
+			u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC,
+					ISPCCDC_SYN_MODE);
+			fid = syn_mode & ISPCCDC_SYN_MODE_FLDSTAT;
+			/* toggle the software maintained fid */
+			ccdc->fldstat ^= 1;
+			if (fid == ccdc->fldstat) {
+				if (fid == 0) {
+					restart = ccdc_isr_buffer(ccdc);
+					goto done;
+				}
+			} else if (fid == 0) {
+				ccdc->fldstat = fid;
+			}
+		} else {
+			restart = ccdc_isr_buffer(ccdc);
+		}
+	}
+done:
 	spin_lock_irqsave(&ccdc->lock, flags);
 	if (__ccdc_handle_stopping(ccdc, CCDC_EVENT_VD0)) {
 		spin_unlock_irqrestore(&ccdc->lock, flags);
@@ -1610,7 +1690,8 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device *ccdc, u32 events)
 	if (ccdc->state == ISP_PIPELINE_STREAM_STOPPED)
 		return 0;
 
-	if (events & IRQ0STATUS_CCDC_VD1_IRQ)
+	if (!ccdc_input_is_bt656(ccdc) &&
+			(events & IRQ0STATUS_CCDC_VD1_IRQ))
 		ccdc_vd1_isr(ccdc);
 
 	ccdc_lsc_isr(ccdc, events);
@@ -1618,7 +1699,8 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device *ccdc, u32 events)
 	if (events & IRQ0STATUS_CCDC_VD0_IRQ)
 		ccdc_vd0_isr(ccdc);
 
-	if (events & IRQ0STATUS_HS_VS_IRQ)
+	if (!ccdc_input_is_bt656(ccdc) &&
+			(events & IRQ0STATUS_HS_VS_IRQ))
 		ccdc_hs_vs_isr(ccdc);
 
 	return 0;
@@ -1728,7 +1810,7 @@ static int ccdc_set_stream(struct v4l2_subdev *sd, int enable)
 		 * links are inactive.
 		 */
 		ccdc_config_vp(ccdc);
-		ccdc_enable_vp(ccdc, 1);
+		ccdc_enable_vp(ccdc, 0);
 		ccdc->error = 0;
 		ccdc_print_status(ccdc);
 	}
@@ -2278,7 +2360,7 @@ int omap3isp_ccdc_init(struct isp_device *isp)
 
 	ccdc->vpcfg.pixelclk = 0;
 
-	ccdc->update = OMAP3ISP_CCDC_BLCLAMP;
+	ccdc->update = 0;
 	ccdc_apply_controls(ccdc);
 
 	ret = ccdc_init_entities(ccdc);
diff --git a/drivers/media/video/omap3isp/ispccdc.h b/drivers/media/video/omap3isp/ispccdc.h
index 54811ce..7f933b2 100644
--- a/drivers/media/video/omap3isp/ispccdc.h
+++ b/drivers/media/video/omap3isp/ispccdc.h
@@ -159,6 +159,7 @@ struct isp_ccdc_device {
 	struct ispccdc_vp vpcfg;
 
 	unsigned int underrun:1;
+	unsigned int fldstat:1;
 	enum isp_pipeline_stream_state state;
 	spinlock_t lock;
 	wait_queue_head_t wait;
diff --git a/drivers/media/video/omap3isp/ispreg.h b/drivers/media/video/omap3isp/ispreg.h
index 69f6af6..ada39c6 100644
--- a/drivers/media/video/omap3isp/ispreg.h
+++ b/drivers/media/video/omap3isp/ispreg.h
@@ -827,6 +827,7 @@
 #define ISPCCDC_SDOFST_LOFST2_SHIFT		3
 #define ISPCCDC_SDOFST_LOFST1_SHIFT		6
 #define ISPCCDC_SDOFST_LOFST0_SHIFT		9
+#define ISPCCDC_SDOFST_LOFST_MASK              0x7
 #define EVENEVEN				1
 #define ODDEVEN					2
 #define EVENODD					3
diff --git a/drivers/media/video/omap3isp/ispvideo.c b/drivers/media/video/omap3isp/ispvideo.c
index cc73375..d59f886 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -164,7 +164,7 @@ static bool isp_video_is_shiftable(enum v4l2_mbus_pixelcode in,
  *
  * Return the number of padding bytes at end of line.
  */
-static unsigned int isp_video_mbus_to_pix(const struct isp_video *video,
+unsigned int isp_video_mbus_to_pix(const struct isp_video *video,
 					  const struct v4l2_mbus_framefmt *mbus,
 					  struct v4l2_pix_format *pix)
 {
diff --git a/drivers/media/video/omap3isp/ispvideo.h b/drivers/media/video/omap3isp/ispvideo.h
index 52fe46b..5fa8fd7 100644
--- a/drivers/media/video/omap3isp/ispvideo.h
+++ b/drivers/media/video/omap3isp/ispvideo.h
@@ -200,7 +200,9 @@ struct isp_buffer *omap3isp_video_buffer_next(struct isp_video *video,
 					      unsigned int error);
 void omap3isp_video_resume(struct isp_video *video, int continuous);
 struct media_pad *omap3isp_video_remote_pad(struct isp_video *video);
-
+extern unsigned int isp_video_mbus_to_pix(const struct isp_video *video,
+				  const struct v4l2_mbus_framefmt *mbus,
+				  struct v4l2_pix_format *pix);
 const struct isp_format_info *
 omap3isp_video_format_info(enum v4l2_mbus_pixelcode code);
 
-- 
1.7.4.1


[-- Attachment #3: 0002-igep00x0-remove-fldmode-from-camera-platform-data.patch --]
[-- Type: text/x-patch, Size: 1029 bytes --]

From 3e5c74f68412ab07e8303f712a2fc7c33ab0d843 Mon Sep 17 00:00:00 2001
From: Enrico Butera <ebutera@users.berlios.de>
Date: Mon, 10 Oct 2011 14:34:33 +0200
Subject: [PATCH] igep00x0: remove fldmode from camera platform data

Signed-off-by: Enrico Butera <ebutera@users.berlios.de>
---
 arch/arm/mach-omap2/board-igep00x0.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/board-igep00x0.c b/arch/arm/mach-omap2/board-igep00x0.c
index 6125057..74fd18c 100644
--- a/arch/arm/mach-omap2/board-igep00x0.c
+++ b/arch/arm/mach-omap2/board-igep00x0.c
@@ -580,7 +580,6 @@ static struct isp_v4l2_subdevs_group igep00x0_camera_subdevs[] = {
                                 .clk_pol                = 0,
                                 .hs_pol                 = 0,
                                 .vs_pol                 = 0,
-                                .fldmode                = 1,
                                 .bt656               = 1,
                 } },
         },
-- 
1.7.4.1

Re: omap3-isp status

[Enrico]

On Mon, Oct 10, 2011 at 12:33 PM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Mon, Oct 10, 2011 at 12:07 PM, Enrico <ebutera@users.berlios.de> wrote:
>> On Mon, Oct 10, 2011 at 12:06 PM, Enrico <ebutera@users.berlios.de> wrote:
>>> I have updated my igep openembedded layer at [1] (testing branch) with:
>>
>> Ops, forgot [1] !
>>
>> [1]: https://github.com/ebutera/meta-igep
>>
>> Enrico
>>
>
> Perfect, thank you Enrico. I will try this latter today and let you
> know. I'm sure I can get this working (with the ghosting effect of
> course) so you can at least obtain 25 fps and once I have this working
> I will resend the patch-set as v3 so Laurent can review it and
> hopefully help us to fix the artifact on the video.

For your tests i suggest to add "nohlt" to the kernel cmdline, see [1]
for the reason.

Enrico

[1]: http://www.spinics.net/lists/linux-media/msg37795.html

Re: omap3-isp status

[Enrico]

On Mon, Oct 10, 2011 at 2:46 PM, Enrico <ebutera@users.berlios.de> wrote:
> On Mon, Oct 10, 2011 at 12:33 PM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> On Mon, Oct 10, 2011 at 12:07 PM, Enrico <ebutera@users.berlios.de> wrote:
>>> On Mon, Oct 10, 2011 at 12:06 PM, Enrico <ebutera@users.berlios.de> wrote:
>>>> I have updated my igep openembedded layer at [1] (testing branch) with:
>>>
>>> Ops, forgot [1] !
>>>
>>> [1]: https://github.com/ebutera/meta-igep
>>>
>>> Enrico
>>>
>>
>> Perfect, thank you Enrico. I will try this latter today and let you
>> know. I'm sure I can get this working (with the ghosting effect of
>> course) so you can at least obtain 25 fps and once I have this working
>> I will resend the patch-set as v3 so Laurent can review it and
>> hopefully help us to fix the artifact on the video.
>
> Yes it must be something simple but not easy to spot.
>
> And to add even more confusion i've attached two patches: one is a
> port of two Deepthy patches, the other one just a board fix.
>
> Just replace patches 0017-18-19 with the attached 0001 patch, and
> after patch 0025 apply the attached 0002 patch.
>
> With these i can succesfully grab frames with yavta BUT i only get
> half-height frames. Disclaimer: i just made the patch monkey and gave
> it a run without a review so it could be anything.

My bad, i forgot a part about config_outlineoffset (ODDEVEN...), i
still have (different) half-green images though...

Side note: while making some tests i can confirm that the solution
adopted in Deepthy patches:

u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
fid = syn_mode & ISPCCDC_SYN_MODE_FLDSTAT;
/* toggle the software maintained fid */

works as expected, i see fid switching correctly.

Enrico

Re: omap3-isp status

[Enrico]

On Mon, Oct 10, 2011 at 4:17 PM, Enrico <ebutera@users.berlios.de> wrote:
> On Mon, Oct 10, 2011 at 2:46 PM, Enrico <ebutera@users.berlios.de> wrote:
>> On Mon, Oct 10, 2011 at 12:33 PM, Javier Martinez Canillas
>>> Perfect, thank you Enrico. I will try this latter today and let you
>>> know. I'm sure I can get this working (with the ghosting effect of
>>> course) so you can at least obtain 25 fps and once I have this working
>>> I will resend the patch-set as v3 so Laurent can review it and
>>> hopefully help us to fix the artifact on the video.
>>
>> Yes it must be something simple but not easy to spot.
>>
>> And to add even more confusion i've attached two patches: one is a
>> port of two Deepthy patches, the other one just a board fix.
>>
>> Just replace patches 0017-18-19 with the attached 0001 patch, and
>> after patch 0025 apply the attached 0002 patch.
>>
>> With these i can succesfully grab frames with yavta BUT i only get
>> half-height frames. Disclaimer: i just made the patch monkey and gave
>> it a run without a review so it could be anything.
>
> My bad, i forgot a part about config_outlineoffset (ODDEVEN...), i
> still have (different) half-green images though...
>
> Side note: while making some tests i can confirm that the solution
> adopted in Deepthy patches:
>
> u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
> fid = syn_mode & ISPCCDC_SYN_MODE_FLDSTAT;
> /* toggle the software maintained fid */
>
> works as expected, i see fid switching correctly.

Ok, i made it work. It was missing just the config_outlineoffset i
wrote before and a missing FLDMODE in SYNC registers.

Moreover it seems to me that the software-maintained field id
(interlaced_cnt in Javier patches, fldstat in Deepthy patches) is
useless, i've tried to only use the FLDSTAT bit from isp register
(fid) in vd0_isr:

if (fid == 0) {
     restart = ccdc_isr_buffer(ccdc);
     goto done;
}

and it works. I've not tested very long frame sequences, only up to 16
frames. The only issue is that the first frame could be half-green
because a field is missing.

Enrico

Re: omap3-isp status

[Javier Martinez Canillas]

On Mon, Oct 10, 2011 at 6:34 PM, Enrico <ebutera@users.berlios.de> wrote:
> On Mon, Oct 10, 2011 at 4:17 PM, Enrico <ebutera@users.berlios.de> wrote:
>> On Mon, Oct 10, 2011 at 2:46 PM, Enrico <ebutera@users.berlios.de> wrote:
>>> On Mon, Oct 10, 2011 at 12:33 PM, Javier Martinez Canillas
>>>> Perfect, thank you Enrico. I will try this latter today and let you
>>>> know. I'm sure I can get this working (with the ghosting effect of
>>>> course) so you can at least obtain 25 fps and once I have this working
>>>> I will resend the patch-set as v3 so Laurent can review it and
>>>> hopefully help us to fix the artifact on the video.
>>>
>>> Yes it must be something simple but not easy to spot.
>>>
>>> And to add even more confusion i've attached two patches: one is a
>>> port of two Deepthy patches, the other one just a board fix.
>>>
>>> Just replace patches 0017-18-19 with the attached 0001 patch, and
>>> after patch 0025 apply the attached 0002 patch.
>>>
>>> With these i can succesfully grab frames with yavta BUT i only get
>>> half-height frames. Disclaimer: i just made the patch monkey and gave
>>> it a run without a review so it could be anything.
>>
>> My bad, i forgot a part about config_outlineoffset (ODDEVEN...), i
>> still have (different) half-green images though...
>>
>> Side note: while making some tests i can confirm that the solution
>> adopted in Deepthy patches:
>>
>> u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
>> fid = syn_mode & ISPCCDC_SYN_MODE_FLDSTAT;
>> /* toggle the software maintained fid */
>>
>> works as expected, i see fid switching correctly.
>
> Ok, i made it work. It was missing just the config_outlineoffset i
> wrote before and a missing FLDMODE in SYNC registers.
>

Great, do you get the ghosting effect or do you have a clean video?

> Moreover it seems to me that the software-maintained field id
> (interlaced_cnt in Javier patches, fldstat in Deepthy patches) is
> useless, i've tried to only use the FLDSTAT bit from isp register
> (fid) in vd0_isr:
>
> if (fid == 0) {
>     restart = ccdc_isr_buffer(ccdc);
>     goto done;
> }
>
> and it works. I've not tested very long frame sequences, only up to 16
> frames. The only issue is that the first frame could be half-green
> because a field is missing.
>
> Enrico
>

Yes, when I tried Deepthy patches I realized that the fldstat was not
in sync with the frames, but probably I made something wrong.

We had the same problem with the hal-green frame. Our solution was to
synchronize the CCDC with the first even field looking at fdstat on
the VD1 interrupt handler and forcing to start processing from an ODD
sub-frame.

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Enrico]

On Mon, Oct 10, 2011 at 6:53 PM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Mon, Oct 10, 2011 at 6:34 PM, Enrico <ebutera@users.berlios.de> wrote:
>> Ok, i made it work. It was missing just the config_outlineoffset i
>> wrote before and a missing FLDMODE in SYNC registers.
>>
>
> Great, do you get the ghosting effect or do you have a clean video?


Unfortunately i always get the ghosting effect. But this is something
we will try to fix later.


>> Moreover it seems to me that the software-maintained field id
>> (interlaced_cnt in Javier patches, fldstat in Deepthy patches) is
>> useless, i've tried to only use the FLDSTAT bit from isp register
>> (fid) in vd0_isr:
>>
>> if (fid == 0) {
>>     restart = ccdc_isr_buffer(ccdc);
>>     goto done;
>> }
>>
>> and it works. I've not tested very long frame sequences, only up to 16
>> frames. The only issue is that the first frame could be half-green
>> because a field is missing.
>>
>
> Yes, when I tried Deepthy patches I realized that the fldstat was not
> in sync with the frames, but probably I made something wrong.


I had noticed the same thing, but now i tested it and it is ok, maybe
my fault too.


> We had the same problem with the hal-green frame. Our solution was to
> synchronize the CCDC with the first even field looking at fdstat on
> the VD1 interrupt handler and forcing to start processing from an ODD
> sub-frame.

Thinking more about it, it's ugly to have that half-green video frame
even if it's just one. It's better to keep your or Deepthy solution.

Enrico

Re: omap3-isp status

[Javier Martinez Canillas]

On Mon, Oct 10, 2011 at 7:09 PM, Enrico <ebutera@users.berlios.de> wrote:
> On Mon, Oct 10, 2011 at 6:53 PM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> On Mon, Oct 10, 2011 at 6:34 PM, Enrico <ebutera@users.berlios.de> wrote:
>>> Ok, i made it work. It was missing just the config_outlineoffset i
>>> wrote before and a missing FLDMODE in SYNC registers.
>>>
>>
>> Great, do you get the ghosting effect or do you have a clean video?
>
>
> Unfortunately i always get the ghosting effect. But this is something
> we will try to fix later.
>
>

Agree, we should try to get some code upstream to add interlaced video
and bt.656 support and fix the artifact later.

>>> Moreover it seems to me that the software-maintained field id
>>> (interlaced_cnt in Javier patches, fldstat in Deepthy patches) is
>>> useless, i've tried to only use the FLDSTAT bit from isp register
>>> (fid) in vd0_isr:
>>>
>>> if (fid == 0) {
>>>     restart = ccdc_isr_buffer(ccdc);
>>>     goto done;
>>> }
>>>
>>> and it works. I've not tested very long frame sequences, only up to 16
>>> frames. The only issue is that the first frame could be half-green
>>> because a field is missing.
>>>
>>
>> Yes, when I tried Deepthy patches I realized that the fldstat was not
>> in sync with the frames, but probably I made something wrong.
>
>
> I had noticed the same thing, but now i tested it and it is ok, maybe
> my fault too.
>
>
>> We had the same problem with the hal-green frame. Our solution was to
>> synchronize the CCDC with the first even field looking at fdstat on
>> the VD1 interrupt handler and forcing to start processing from an ODD
>> sub-frame.
>
> Thinking more about it, it's ugly to have that half-green video frame
> even if it's just one. It's better to keep your or Deepthy solution.
>
> Enrico
>

Well, that is something that can be fixed later also. Can you send to
the list your patches? So, Laurent, Sakari and others than know more
about the ISP can review it. I hope they can find the cause for the
artifact.

Thank you and best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain

Re: omap3-isp status

[Enrico]

[-- Attachment #1: Type: text/plain, Size: 2867 bytes --]

On Mon, Oct 10, 2011 at 8:18 PM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Mon, Oct 10, 2011 at 7:09 PM, Enrico <ebutera@users.berlios.de> wrote:
>> On Mon, Oct 10, 2011 at 6:53 PM, Javier Martinez Canillas
>> <martinez.javier@gmail.com> wrote:
>>> On Mon, Oct 10, 2011 at 6:34 PM, Enrico <ebutera@users.berlios.de> wrote:
>>>> Ok, i made it work. It was missing just the config_outlineoffset i
>>>> wrote before and a missing FLDMODE in SYNC registers.
>>>>
>>>
>>> Great, do you get the ghosting effect or do you have a clean video?
>>
>>
>> Unfortunately i always get the ghosting effect. But this is something
>> we will try to fix later.
>>
>>
>
> Agree, we should try to get some code upstream to add interlaced video
> and bt.656 support and fix the artifact later.
>
>>>> Moreover it seems to me that the software-maintained field id
>>>> (interlaced_cnt in Javier patches, fldstat in Deepthy patches) is
>>>> useless, i've tried to only use the FLDSTAT bit from isp register
>>>> (fid) in vd0_isr:
>>>>
>>>> if (fid == 0) {
>>>>     restart = ccdc_isr_buffer(ccdc);
>>>>     goto done;
>>>> }
>>>>
>>>> and it works. I've not tested very long frame sequences, only up to 16
>>>> frames. The only issue is that the first frame could be half-green
>>>> because a field is missing.
>>>>
>>>
>>> Yes, when I tried Deepthy patches I realized that the fldstat was not
>>> in sync with the frames, but probably I made something wrong.
>>
>>
>> I had noticed the same thing, but now i tested it and it is ok, maybe
>> my fault too.
>>
>>
>>> We had the same problem with the hal-green frame. Our solution was to
>>> synchronize the CCDC with the first even field looking at fdstat on
>>> the VD1 interrupt handler and forcing to start processing from an ODD
>>> sub-frame.
>>
>> Thinking more about it, it's ugly to have that half-green video frame
>> even if it's just one. It's better to keep your or Deepthy solution.
>>
>> Enrico
>>
>
> Well, that is something that can be fixed later also. Can you send to
> the list your patches? So, Laurent, Sakari and others than know more
> about the ISP can review it. I hope they can find the cause for the
> artifact.

I'm attaching some fixes (taken from Deepthy patches) to be applied on
top of your v2 patches, with those i can grab frames but i only get
garbage.

I think the problem is that it always hits this in ccdc_isr_buffer:

if (ccdc_sbl_wait_idle(ccdc, 1000)) {
                dev_info(isp->dev, "CCDC won't become idle!\n");
                goto done;
}

so the video buffer never gets updated.

At this point i think it is better to go on with my port of Deepthy
patches and try to solve the ghosting issues, maybe with your fixes
about buffer decoupling.

Laurent, what do you suggest to do?

Enrico

[-- Attachment #2: 0001-omap3isp-some-fixes-for-Javier-v2-patches.patch --]
[-- Type: text/x-patch, Size: 5408 bytes --]

From 078f2843ba94cfbc150f6c01cc614c0ed3a35fd4 Mon Sep 17 00:00:00 2001
From: Enrico Butera <ebutera@users.berlios.de>
Date: Tue, 11 Oct 2011 11:51:38 +0200
Subject: [PATCH] omap3isp: some fixes for Javier v2 patches

Signed-off-by: Enrico Butera <ebutera@users.berlios.de>
---
 drivers/media/video/omap3isp/ispccdc.c |   41 +++++++++++++++++++++++--------
 1 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index f1da49c..0cb4e36 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -43,6 +43,7 @@ __ccdc_get_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh,
 		  unsigned int pad, enum v4l2_subdev_format_whence which);
 static bool ccdc_input_is_bt656(struct isp_ccdc_device *ccdc);
 static bool ccdc_input_is_fldmode(struct isp_ccdc_device *ccdc);
+static int __ccdc_handle_stopping(struct isp_ccdc_device *ccdc, u32 event);
 
 static const unsigned int ccdc_fmts[] = {
 	V4L2_MBUS_FMT_Y8_1X8,
@@ -795,11 +796,17 @@ static void ccdc_apply_controls(struct isp_ccdc_device *ccdc)
 void omap3isp_ccdc_restore_context(struct isp_device *isp)
 {
 	struct isp_ccdc_device *ccdc = &isp->isp_ccdc;
+	struct v4l2_mbus_framefmt *format;
 
 	isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG, ISPCCDC_CFG_VDLC);
 
-	ccdc->update = OMAP3ISP_CCDC_ALAW | OMAP3ISP_CCDC_LPF
-		     | OMAP3ISP_CCDC_BLCLAMP | OMAP3ISP_CCDC_BCOMP;
+	/* CCDC_PAD_SINK */
+	format = &ccdc->formats[CCDC_PAD_SINK];
+	if ((format->code != V4L2_MBUS_FMT_UYVY8_2X8) &&
+			(format->code != V4L2_MBUS_FMT_UYVY8_2X8))
+		ccdc->update = OMAP3ISP_CCDC_ALAW | OMAP3ISP_CCDC_LPF
+				| OMAP3ISP_CCDC_BLCLAMP | OMAP3ISP_CCDC_BCOMP;
+
 	ccdc_apply_controls(ccdc);
 	ccdc_configure_fpc(ccdc);
 }
@@ -1021,10 +1028,10 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 
 	if (pdata && pdata->bt656)
 		isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF,
-			    ISPCCDC_REC656IF_R656ON);
+			    ISPCCDC_REC656IF_R656ON | ISPCCDC_REC656IF_ECCFVH);
 	else
 		isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF,
-			    ISPCCDC_REC656IF_R656ON);
+			    ISPCCDC_REC656IF_R656ON | ISPCCDC_REC656IF_ECCFVH);
 }
 
 /* CCDC formats descriptions */
@@ -1187,7 +1194,9 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 		ccdc_pattern = ccdc_sgrbg_pattern;
 		break;
 	}
-	ccdc_config_imgattr(ccdc, ccdc_pattern);
+	if ((format->code != V4L2_MBUS_FMT_YUYV8_2X8) &&
+			(format->code != V4L2_MBUS_FMT_UYVY8_2X8))
+		ccdc_config_imgattr(ccdc, ccdc_pattern);
 
 	/* In BT.656 a pixel is representd using two bytes */
 	if (pdata->bt656)
@@ -1219,7 +1228,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HORZ_INFO);
 	isp_reg_writel(isp, 0 << ISPCCDC_VERT_START_SLV0_SHIFT,
 		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_START);
-	isp_reg_writel(isp, nlv << ISPCCDC_VERT_LINES_NLV_SHIFT,
+	isp_reg_writel(isp, (((format->height >> 1) - 1) /*nlv*/ << ISPCCDC_VERT_LINES_NLV_SHIFT),
 		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_LINES);
 	isp_reg_writel(isp, 0 << ISPCCDC_VERT_START_SLV1_SHIFT,
 		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_START);
@@ -1278,6 +1287,11 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 unlock:
 	spin_unlock_irqrestore(&ccdc->lsc.req_lock, flags);
 
+	if (pdata->bt656)
+		ccdc->update = OMAP3ISP_CCDC_BLCLAMP;
+	else
+		ccdc->update = 0;
+
 	ccdc_apply_controls(ccdc);
 }
 
@@ -1299,6 +1313,11 @@ static int ccdc_disable(struct isp_ccdc_device *ccdc)
 		ccdc->stopping = CCDC_STOP_REQUEST;
 	spin_unlock_irqrestore(&ccdc->lock, flags);
 
+	__ccdc_lsc_enable(ccdc, 0);
+	__ccdc_enable(ccdc, 0);
+	ccdc->stopping = CCDC_STOP_EXECUTED;
+	__ccdc_handle_stopping(ccdc, CCDC_STOP_FINISHED);
+
 	ret = wait_event_timeout(ccdc->wait,
 				 ccdc->stopping == CCDC_STOP_FINISHED,
 				 msecs_to_jiffies(2000));
@@ -1522,7 +1541,7 @@ static int ccdc_isr_buffer(struct isp_ccdc_device *ccdc)
 	/* In interlaced mode a frame is composed of two subframes so we don't have
 	 * to change the CCDC output memory on every end of frame.
 	 */
-	if (!ccdc_input_is_fldmode(ccdc)) {
+	if (ccdc_input_is_fldmode(ccdc)) {
 		if (!ccdc->interlaced_cnt) {
 			ccdc->interlaced_cnt = 1;
 			restart = 1;
@@ -1656,7 +1675,7 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device *ccdc, u32 events)
 	if (ccdc->state == ISP_PIPELINE_STREAM_STOPPED)
 		return 0;
 
-	if (events & IRQ0STATUS_CCDC_VD1_IRQ)
+	if ((events & IRQ0STATUS_CCDC_VD1_IRQ) && !ccdc_input_is_bt656(ccdc))
 		ccdc_vd1_isr(ccdc);
 
 	ccdc_lsc_isr(ccdc, events);
@@ -1664,7 +1683,7 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device *ccdc, u32 events)
 	if (events & IRQ0STATUS_CCDC_VD0_IRQ)
 		ccdc_vd0_isr(ccdc);
 
-	if (events & IRQ0STATUS_HS_VS_IRQ)
+	if ((events & IRQ0STATUS_HS_VS_IRQ) && !ccdc_input_is_bt656(ccdc))
 		ccdc_hs_vs_isr(ccdc);
 
 	return 0;
@@ -1774,7 +1793,7 @@ static int ccdc_set_stream(struct v4l2_subdev *sd, int enable)
 		 * links are inactive.
 		 */
 		ccdc_config_vp(ccdc);
-		ccdc_enable_vp(ccdc, 1);
+		ccdc_enable_vp(ccdc, 0);
 		ccdc->error = 0;
 		ccdc_print_status(ccdc);
 	}
@@ -2344,7 +2363,7 @@ int omap3isp_ccdc_init(struct isp_device *isp)
 
 	ccdc->vpcfg.pixelclk = 0;
 
-	ccdc->update = OMAP3ISP_CCDC_BLCLAMP;
+	ccdc->update = 0;
 	ccdc_apply_controls(ccdc);
 
 	ret = ccdc_init_entities(ccdc);
-- 
1.7.4.1

Re: omap3-isp status

[Javier Martinez Canillas]

On Tue, Oct 11, 2011 at 12:29 PM, Enrico <ebutera@users.berlios.de> wrote:
> On Mon, Oct 10, 2011 at 8:18 PM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> On Mon, Oct 10, 2011 at 7:09 PM, Enrico <ebutera@users.berlios.de> wrote:
>>> On Mon, Oct 10, 2011 at 6:53 PM, Javier Martinez Canillas
>>> <martinez.javier@gmail.com> wrote:
>>>> On Mon, Oct 10, 2011 at 6:34 PM, Enrico <ebutera@users.berlios.de> wrote:
>>>>> Ok, i made it work. It was missing just the config_outlineoffset i
>>>>> wrote before and a missing FLDMODE in SYNC registers.
>>>>>
>>>>
>>>> Great, do you get the ghosting effect or do you have a clean video?
>>>
>>>
>>> Unfortunately i always get the ghosting effect. But this is something
>>> we will try to fix later.
>>>
>>>
>>
>> Agree, we should try to get some code upstream to add interlaced video
>> and bt.656 support and fix the artifact later.
>>
>>>>> Moreover it seems to me that the software-maintained field id
>>>>> (interlaced_cnt in Javier patches, fldstat in Deepthy patches) is
>>>>> useless, i've tried to only use the FLDSTAT bit from isp register
>>>>> (fid) in vd0_isr:
>>>>>
>>>>> if (fid == 0) {
>>>>>     restart = ccdc_isr_buffer(ccdc);
>>>>>     goto done;
>>>>> }
>>>>>
>>>>> and it works. I've not tested very long frame sequences, only up to 16
>>>>> frames. The only issue is that the first frame could be half-green
>>>>> because a field is missing.
>>>>>
>>>>
>>>> Yes, when I tried Deepthy patches I realized that the fldstat was not
>>>> in sync with the frames, but probably I made something wrong.
>>>
>>>
>>> I had noticed the same thing, but now i tested it and it is ok, maybe
>>> my fault too.
>>>
>>>
>>>> We had the same problem with the hal-green frame. Our solution was to
>>>> synchronize the CCDC with the first even field looking at fdstat on
>>>> the VD1 interrupt handler and forcing to start processing from an ODD
>>>> sub-frame.
>>>
>>> Thinking more about it, it's ugly to have that half-green video frame
>>> even if it's just one. It's better to keep your or Deepthy solution.
>>>
>>> Enrico
>>>
>>
>> Well, that is something that can be fixed later also. Can you send to
>> the list your patches? So, Laurent, Sakari and others than know more
>> about the ISP can review it. I hope they can find the cause for the
>> artifact.
>
> I'm attaching some fixes (taken from Deepthy patches) to be applied on
> top of your v2 patches, with those i can grab frames but i only get
> garbage.
>

Yes, I still couldn't find time to try that patch on real hardware, sorry.

> I think the problem is that it always hits this in ccdc_isr_buffer:
>
> if (ccdc_sbl_wait_idle(ccdc, 1000)) {
>                dev_info(isp->dev, "CCDC won't become idle!\n");
>                goto done;
> }
>
> so the video buffer never gets updated.
>
> At this point i think it is better to go on with my port of Deepthy
> patches and try to solve the ghosting issues, maybe with your fixes
> about buffer decoupling.
>

I second you on that. Deepthy patches make two changes to the ISP in
order to support BT.656:

1- Add support to configure the bridge with UYVY8_2X8 format.
2- Add support to interlaced video mode.

Looking at your fixes taken from Deepthy I see code that still make
(1). I understood from Laurent's comments that its oma3isp-yuv branch
already made (1), but he also said he didn't try on hardware though.

I think the best approach is to resend Deepthy patches again on top of
Laurent's yuv branch so it can be reviewed. Probably Laurent will want
then to split the patch in two, one to fix the UYVY8_2X8 support and
another one that adds support to interlaced video mode.

> Laurent, what do you suggest to do?
>
> Enrico
>

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain