omap3-isp status

Message ID CAAwP0s3NFvvUYd-0kwKLKXfYB4Zx1nXb0nd9+JM61JWtrVFfRg@mail.gmail.com (mailing list archive)
State RFC, archived
Headers

Commit Message

Javier Martinez Canillas Oct. 9, 2011, 11 p.m. UTC
  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?

        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,
  

Comments

Enrico Oct. 10, 2011, 8:54 a.m. UTC | #1
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
--
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, 9:02 a.m. UTC | #2
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,
  
Enrico Oct. 10, 2011, 10:06 a.m. UTC | #3
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
--
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, 10:07 a.m. UTC | #4
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
--
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, 10:33 a.m. UTC | #5
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,
  
Enrico Oct. 10, 2011, 12:54 p.m. UTC | #6
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
--
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
  

Patch

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