omap3-isp status

Message ID CA+2YH7s6rhLsyJTdWwQVUCd2WBWiH2saSaZZw0tysRWsXw-6Cg@mail.gmail.com (mailing list archive)
State RFC, archived
Headers

Commit Message

Enrico Oct. 10, 2011, 12:46 p.m. UTC
  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
  

Comments

Enrico Oct. 10, 2011, 2:17 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Enrico Oct. 10, 2011, 4:34 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Javier Martinez Canillas Oct. 10, 2011, 4:53 p.m. UTC | #3
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,
  
Enrico Oct. 10, 2011, 5:09 p.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Javier Martinez Canillas Oct. 10, 2011, 6:18 p.m. UTC | #5
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,
  

Patch

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