omapfb: help from userspace

omapfb: help from userspace

[Felipe Contreras]

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

Hi,

I've been trying to get the yuv overlay to work, but without success.
I'm trying to do what Mans Rullgard is doing on omapfbplay[1], but I
don't see anything happening.

The normal RGB plane is working fine, I'm attaching a test that writes
data to the whole framebuffer.

This is the output:
# /tmp/fb_test
main: size=720x400, 16bpp
overlay: size=720x400, 16bpp
panel: enabled=0
panel: 0x0 - 720x400
# /tmp/fb_test
main: size=720x400, 16bpp
overlay: size=720x400, 16bpp
panel: enabled=1
panel: 0x0 - 720x400

Best regards.

[1] http://git.mansr.com/?p=omapfbplay

-- 
Felipe Contreras

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fb_test.c --]
[-- Type: text/x-csrc; name=fb_test.c, Size: 4027 bytes --]

#include <fcntl.h>
#include <sys/ioctl.h>
#include <linux/fb.h>
#include <sys/mman.h>

#include <mach/omapfb.h>

#include <stdio.h>
#include <stdint.h>
#include <stdarg.h>

#define MIN(a, b) (((a) < (b)) ? (a) : (b))

static int main_fb;
static int overlay_fb;

static struct fb_var_screeninfo main_screen_info;
static struct fb_var_screeninfo overlay_screen_info;
static struct omapfb_mem_info mem_info;
static struct omapfb_plane_info plane_info;

uint8_t *overlay_mem;

unsigned int input_width = 640;
unsigned int input_height = 480;

static void
log_helper (const char *type,
            const char *file,
            const char *function,
            unsigned int line,
            const char *fmt,
            ...)
{
    char *tmp = NULL;
    va_list args;

    va_start (args, fmt);

    vasprintf (&tmp, fmt, args);

    fprintf (stderr, "%s %s:%d:%s() %s\n",
             type,
             file, line, function,
             tmp);

    free (tmp);

    va_end (args);
}

#define log_error(...) log_helper ("error", __FILE__, __func__, __LINE__, __VA_ARGS__);

void
set_screen (void)
{
#if 0
    overlay_screen_info.xres = MIN (main_screen_info.xres, input_width) & ~15;
    overlay_screen_info.yres = MIN (main_screen_info.yres, input_height) & ~15;
    overlay_screen_info.xoffset = 0;
    overlay_screen_info.yoffset = 0;
#else
    overlay_screen_info.xres = main_screen_info.xres;
    overlay_screen_info.yres = main_screen_info.yres;
#endif
    overlay_screen_info.nonstd = OMAPFB_COLOR_YUY422;

    {
        int err;
        err = ioctl (overlay_fb, FBIOPUT_VSCREENINFO, &overlay_screen_info);
        if (err != 0)
        {
            log_error ("overlay set");
            return;
        }
    }
}

void
set_plane (void)
{
    plane_info.enabled = 1;
#if 0
    plane_info.pos_x = 0;
    plane_info.pos_y = 0;
    plane_info.out_width = main_screen_info.xres;
    plane_info.out_height = main_screen_info.yres;
#endif

    {
        int err;
        err = ioctl (overlay_fb, OMAPFB_SETUP_PLANE, &plane_info);
        if (err != 0)
        {
            log_error ("error=%i", err);
            return;
        }
    }
}

void
clean_omap (void)
{
    unsigned int i;
    for (i = 0; i < mem_info.size / 4; i++)
        ((uint32_t *) overlay_mem)[i] = 0xFFFFAAAA;
}

int
main (void)
{
    int err;
    main_fb = -1;
    overlay_fb = -1;

    main_fb = open ("/dev/fb0", O_RDWR);

    if (main_fb == -1)
    {
        log_error ("/dev/fb0");
        return 1;
    }

    if (ioctl (main_fb, FBIOGET_VSCREENINFO, &main_screen_info))
    {
        log_error ("main screen");
        goto leave;
    }

    overlay_fb = open ("/dev/fb1", O_RDWR);

    if (overlay_fb == -1)
    {
        log_error ("/dev/fb1");
        return 1;
    }

    if (ioctl (overlay_fb, FBIOGET_VSCREENINFO, &overlay_screen_info))
    {
        log_error ("overlay screen");
        goto leave;
    }

    if (ioctl (overlay_fb, OMAPFB_QUERY_PLANE, &plane_info))
    {
        log_error ("omap pane");
        goto leave;
    }

    if (ioctl (overlay_fb, OMAPFB_QUERY_MEM, &mem_info))
    {
        log_error ("omap mem");
        goto leave;
    }

    printf ("main: size=%ix%i, %ibpp\n", main_screen_info.xres, main_screen_info.yres, main_screen_info.bits_per_pixel);
    printf ("overlay: size=%ix%i, %ibpp\n", overlay_screen_info.xres, overlay_screen_info.yres, overlay_screen_info.bits_per_pixel);

    /* omap */
    printf ("panel: enabled=%u\n", plane_info.enabled);
    printf ("panel: %ux%u - %ux%u\n", plane_info.pos_x, plane_info.pos_y,
            plane_info.out_width, plane_info.out_height);

    /* map the framebuffer */
    overlay_mem = mmap (NULL, mem_info.size, PROT_WRITE, MAP_SHARED, overlay_fb, 0);
    if (overlay_mem == MAP_FAILED)
    {
        log_error ("mmap");
        goto leave;
    }

    clean_omap ();
    set_screen ();
    set_plane ();

leave:
    munmap (overlay_mem, mem_info.size);

    if (overlay_fb)
        close (overlay_fb);

    if (main_fb)
        close (main_fb);

    return 0;
}

Re: omapfb: help from userspace

[Felipe Contreras]

On Mon, Sep 15, 2008 at 5:38 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Hi,
>
> I've been trying to get the yuv overlay to work, but without success.
> I'm trying to do what Mans Rullgard is doing on omapfbplay[1], but I
> don't see anything happening.
>
> The normal RGB plane is working fine, I'm attaching a test that writes
> data to the whole framebuffer.
>
> This is the output:
> # /tmp/fb_test
> main: size=720x400, 16bpp
> overlay: size=720x400, 16bpp
> panel: enabled=0
> panel: 0x0 - 720x400
> # /tmp/fb_test
> main: size=720x400, 16bpp
> overlay: size=720x400, 16bpp
> panel: enabled=1
> panel: 0x0 - 720x400

Nevermind, this patch fixed the issue:
http://gitweb.openembedded.net/?p=org.openembedded.dev.git;a=blob;f=packages/linux/linux-omap2-git/beagleboard/03-enable-overlay-opt.diff;h=9fa749f5fc00a711afc6f31781d45efa885b4ccf;hb=HEAD

Now I'm getting a bunch of these errors apparently when shuffling
memory too fast:
irq -33, desc: c0335cf8, depth: 0, count: 0, unhandled: 0
->handle_irq():  c00744e0, handle_bad_irq+0x0/0x22c
->chip(): 00000000, 0x0
->action(): 00000000

-- 
Felipe Contreras

Re: omapfb: help from userspace

[Måns Rullgård]

"Felipe Contreras" <felipe.contreras@gmail.com> writes:

> On Mon, Sep 15, 2008 at 5:38 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Hi,
>>
>> I've been trying to get the yuv overlay to work, but without success.
>> I'm trying to do what Mans Rullgard is doing on omapfbplay[1], but I
>> don't see anything happening.
>>
>> The normal RGB plane is working fine, I'm attaching a test that writes
>> data to the whole framebuffer.
>>
>> This is the output:
>> # /tmp/fb_test
>> main: size=720x400, 16bpp
>> overlay: size=720x400, 16bpp
>> panel: enabled=0
>> panel: 0x0 - 720x400
>> # /tmp/fb_test
>> main: size=720x400, 16bpp
>> overlay: size=720x400, 16bpp
>> panel: enabled=1
>> panel: 0x0 - 720x400
>
> Nevermind, this patch fixed the issue:
> http://gitweb.openembedded.net/?p=org.openembedded.dev.git;a=blob;f=packages/linux/linux-omap2-git/beagleboard/03-enable-overlay-opt.diff;h=9fa749f5fc00a711afc6f31781d45efa885b4ccf;hb=HEAD

That patch is bad.  Use this one instead:

http://git.mansr.com/?p=linux-omap;a=commitdiff;h=7e052af7e4c73dc450412486ad37eb529e725dc7

It's still not perfect, but it's better.

-- 
Måns Rullgård
mans@mansr.com

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: omapfb: help from userspace

[Nathan Monson]

"Felipe Contreras" <felipe.contreras@gmail.com> writes:
> Now I'm getting a bunch of these errors apparently when shuffling
> memory too fast:
> irq -33, desc: c0335cf8, depth: 0, count: 0, unhandled: 0
> ->handle_irq():  c00744e0, handle_bad_irq+0x0/0x22c
> ->chip(): 00000000, 0x0
> ->action(): 00000000

I've had this problem regularly while using linux-omap .26 and .27 on
the BeagleBoard.  I turned off framebuffer support after seeing your
message and it seemed to help.  (Hard to tell since it is random.)

Now I'm using DSP Bridge and the problem is back.  In fact, the
problem occurs within a minute or two of running code on the DSP.

On the BeagleBoard list, Pratheesh Gangadhar said that mapping I/O
regions as Strongly Ordered suppresses this problem:
http://groups.google.com/group/beagleboard/browse_thread/thread/23e1c95b4bfb09b5/70d12dca569ca503?show_docid=70d12dca569ca503

> This proves that these spurious interrupts are side effect of using posted
> writes to acknowledge/clear the interrupt status in the peripheral.

Is there anyone who knows their way around mmu.c that could create a
quick and dirty patch to use Strongly Ordered I/O on OMAP3?  I'd like
to test it with the DSP.

- Nathan

Re: omapfb: help from userspace

[Nathan Monson]

On Wed, Oct 8, 2008 at 11:36 AM, Nathan Monson <nmonson@gmail.com> wrote:
> "Felipe Contreras" <felipe.contreras@gmail.com> writes:
>> irq -33, desc: c0335cf8, depth: 0, count: 0, unhandled: 0
>
> On the BeagleBoard list, Pratheesh Gangadhar said that mapping I/O
> regions as Strongly Ordered suppresses this problem:
> http://groups.google.com/group/beagleboard/browse_thread/thread/23e1c95b4bfb09b5/70d12dca569ca503?show_docid=70d12dca569ca503

Pratheesh helped me make a patch against the latest linux-omap git to
try this.

With this patch, my IRQ -33 problems with the DSP have disappeared.
Before, I would end up in IRQ -33 loop after 10 invocations of the DSP
Bridge 'ping.out' utility.  I just finished running it 50,000 times
without error.

As stated before, this patch is just a workaround for testing
purposes, not a fix.  Who knows what performance side effects it
has...

---
diff --git a/arch/arm/include/asm/mach/map.h b/arch/arm/include/asm/mach/map.h
index 9eb936e..5cb4f5f 100644
--- a/arch/arm/include/asm/mach/map.h
+++ b/arch/arm/include/asm/mach/map.h
@@ -25,6 +25,7 @@ struct map_desc {
 #define MT_HIGH_VECTORS		8
 #define MT_MEMORY		9
 #define MT_ROM			10
+#define MT_MEMORY_SO		11

 #define MT_NONSHARED_DEVICE	MT_DEVICE_NONSHARED
 #define MT_IXP2000_DEVICE	MT_DEVICE_IXP2000
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index adbe21f..c11c0e8 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -119,13 +119,13 @@ static struct map_desc omap34xx_io_desc[] __initdata = {
 		.virtual	= L3_34XX_VIRT,
 		.pfn		= __phys_to_pfn(L3_34XX_PHYS),
 		.length		= L3_34XX_SIZE,
-		.type		= MT_DEVICE
+		.type		= MT_MEMORY_SO
 	},
 	{
 		.virtual	= L4_34XX_VIRT,
 		.pfn		= __phys_to_pfn(L4_34XX_PHYS),
 		.length		= L4_34XX_SIZE,
-		.type		= MT_DEVICE
+		.type		= MT_MEMORY_SO
 	},
 	{
 		.virtual	= L4_WK_34XX_VIRT,
@@ -137,19 +137,19 @@ static struct map_desc omap34xx_io_desc[] __initdata = {
 		.virtual	= OMAP34XX_GPMC_VIRT,
 		.pfn		= __phys_to_pfn(OMAP34XX_GPMC_PHYS),
 		.length		= OMAP34XX_GPMC_SIZE,
-		.type		= MT_DEVICE
+		.type		= MT_MEMORY_SO
 	},
 	{
 		.virtual	= OMAP343X_SMS_VIRT,
 		.pfn		= __phys_to_pfn(OMAP343X_SMS_PHYS),
 		.length		= OMAP343X_SMS_SIZE,
-		.type		= MT_DEVICE
+		.type		= MT_MEMORY_SO
 	},
 	{
 		.virtual	= OMAP343X_SDRC_VIRT,
 		.pfn		= __phys_to_pfn(OMAP343X_SDRC_PHYS),
 		.length		= OMAP343X_SDRC_SIZE,
-		.type		= MT_DEVICE
+		.type		= MT_MEMORY_SO
 	},
 	{
 		.virtual	= L4_PER_34XX_VIRT,
@@ -161,7 +161,7 @@ static struct map_desc omap34xx_io_desc[] __initdata = {
 		.virtual	= L4_EMU_34XX_VIRT,
 		.pfn		= __phys_to_pfn(L4_EMU_34XX_PHYS),
 		.length		= L4_EMU_34XX_SIZE,
-		.type		= MT_DEVICE
+		.type		= MT_MEMORY_SO
 	},
 };
 #endif
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index a713e40..d5f25ad 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -245,6 +245,10 @@ static struct mem_type mem_types[] = {
 		.prot_sect = PMD_TYPE_SECT,
 		.domain    = DOMAIN_KERNEL,
 	},
+	[MT_MEMORY_SO] = {
+		.prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_SECT_UNCACHED,
+		.domain    = DOMAIN_KERNEL,
+	},
 };

 const struct mem_type *get_mem_type(unsigned int type)
--

RE: omapfb: help from userspace

[Woodruff, Richard]

> As stated before, this patch is just a workaround for testing
> purposes, not a fix.  Who knows what performance side effects it
> has...

The need for this has been discussed on this list a few times in the past.  Not having it is a bug to me for OMAP3.

There shouldn't really be any performance side effects the way it is being used.

Regards,
Richard W.

RE: omapfb: help from userspace

[TK, Pratheesh Gangadhar]

>The need for this has been discussed on this list a few times in the past.  >Not having it is a bug to me for OMAP3.
>
>There shouldn't really be any performance side effects the way it is being >used.

I agree, I did not notice any performance impact - we are using this setting for couple of months now without any problem. But there are some future compatibility concerns in below thread - it should be fine for OMAP3 though 

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg02949.html

> Let's go back to having a strongly ordered memory model.  Please.

On pre-ARMv6, updates to CPSR are guaranteed to take place in program
order with a strongly-ordered memory access. This is still true on
ARMv6/v7 for backward compatibility but the feature is deprecated and it
might not be true for future architecture versions. At some point
barriers might be needed.

Regards,
Pratheesh

Re: omapfb: help from userspace

[Tony Lindgren]

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

* TK, Pratheesh Gangadhar <pratheesh@ti.com> [081009 09:20]:
> >The need for this has been discussed on this list a few times in the past.  >Not having it is a bug to me for OMAP3.
> >
> >There shouldn't really be any performance side effects the way it is being >used.
> 
> I agree, I did not notice any performance impact - we are using this setting for couple of months now without any problem. But there are some future compatibility concerns in below thread - it should be fine for OMAP3 though 
> 
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg02949.html
> 
> > Let's go back to having a strongly ordered memory model.  Please.
> 
> On pre-ARMv6, updates to CPSR are guaranteed to take place in program
> order with a strongly-ordered memory access. This is still true on
> ARMv6/v7 for backward compatibility but the feature is deprecated and it
> might not be true for future architecture versions. At some point
> barriers might be needed.

Nathan, good to hear that the SO mode helps. But since you seem to
have it easily reproducable..

Could you try the following patch based on RMK's earlier patch
without the strongly ordered patch and see if that makes any
difference?

If this patch alone does not do anything, maybe try reading back
something from the dsp interrupt registers after write also
in the dsp interrupt handler?

Also you might want to read through the thread linked in the
patch description.

And, if that still does not work, I'm suspecting that in some cases
write-read is not enough, and only write-write ensures that it gets
posted. Some ARM docs say that writes are only posted relative to
other writes, but I don't know if it can be really that way.

At least we have at least one hack like that in the
drivers/mmc/host/omap.c.

Anyways, it would be nice to solve this issue for good. The problem
has been making it easily reproducable.

Thanks,

Tony

[-- Attachment #2: irq-read.patch --]
[-- Type: text/x-diff, Size: 1056 bytes --]

>From 5732ab12201cff25f036e1e20dc23fdb779c8589 Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Thu, 9 Oct 2008 15:25:16 +0300
Subject: [PATCH] Read INTC_REVISION after write to ensure write is posted

Based on Russell's earlier patch at:

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg02904.html

diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 4ffb4f1..3ae3c8c 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -64,6 +64,7 @@ static u32 intc_bank_read_reg(struct omap_irq_bank *bank, u16 reg)
 static void omap_ack_irq(unsigned int irq)
 {
 	intc_bank_write_reg(0x1, &irq_banks[0], INTC_CONTROL);
+	intc_bank_read_reg(&irq_banks[0], INTC_REVISION);
 }
 
 static void omap_mask_irq(unsigned int irq)
@@ -73,6 +74,7 @@ static void omap_mask_irq(unsigned int irq)
 	irq &= (IRQ_BITS_PER_REG - 1);
 
 	intc_bank_write_reg(1 << irq, &irq_banks[0], INTC_MIR_SET0 + offset);
+	intc_bank_read_reg(&irq_banks[0], INTC_REVISION);
 }
 
 static void omap_unmask_irq(unsigned int irq)

Re: omapfb: help from userspace

[Nathan Monson]

On Thu, Oct 9, 2008 at 5:43 AM, Tony Lindgren <tony@atomide.com> wrote:
> Nathan, good to hear that the SO mode helps. But since you seem to
> have it easily reproducable..

Reproducing is as easy as applying the DSP Bridge kernel patches and
then running the 'ping.out' sample like this:  while true; do
ping.out; done

> Could you try the following patch based on RMK's earlier patch
> without the strongly ordered patch and see if that makes any
> difference?

Maybe it lasted a few seconds longer, or maybe it was luck, but in any
case it eventually went into an IRQ -33 loop even with this patch.

> If this patch alone does not do anything, maybe try reading back
> something from the dsp interrupt registers after write also
> in the dsp interrupt handler?

I will look into this and let you know.

> Also you might want to read through the thread linked in the
> patch description.
>
> And, if that still does not work, I'm suspecting that in some cases
> write-read is not enough, and only write-write ensures that it gets
> posted. Some ARM docs say that writes are only posted relative to
> other writes, but I don't know if it can be really that way.

Should I change the patch you sent me to do write write?

> At least we have at least one hack like that in the
> drivers/mmc/host/omap.c.
>
> Anyways, it would be nice to solve this issue for good. The problem
> has been making it easily reproducable.

At least I have a workaround for now.  It's good to be able to get DSP
work done.

- Nathan

Re: omapfb: help from userspace

[Paul Walmsley]

Hi Nathan,

On Thu, 9 Oct 2008, Nathan Monson wrote:

> On Thu, Oct 9, 2008 at 5:43 AM, Tony Lindgren <tony@atomide.com> wrote:
> > Nathan, good to hear that the SO mode helps. But since you seem to
> > have it easily reproducable..
> 
> Reproducing is as easy as applying the DSP Bridge kernel patches and
> then running the 'ping.out' sample like this:  while true; do
> ping.out; done
> 
> > Could you try the following patch based on RMK's earlier patch
> > without the strongly ordered patch and see if that makes any
> > difference?
> 
> Maybe it lasted a few seconds longer, or maybe it was luck, but in any
> case it eventually went into an IRQ -33 loop even with this patch.

could you try Lauri's patch posted here:

http://marc.info/?l=linux-omap&m=122407150608770&w=2

without the strongly-ordered memory patches?


- Paul

Re: omapfb: help from userspace

[Lauri Leukkunen]

On 15/10/08 05:57 -0600, ext Paul Walmsley wrote:
> Hi Nathan,
> 
> On Thu, 9 Oct 2008, Nathan Monson wrote:
> 
> > On Thu, Oct 9, 2008 at 5:43 AM, Tony Lindgren <tony@atomide.com> wrote:
> > > Nathan, good to hear that the SO mode helps. But since you seem to
> > > have it easily reproducable..
> > 
> > Reproducing is as easy as applying the DSP Bridge kernel patches and
> > then running the 'ping.out' sample like this:  while true; do
> > ping.out; done
> > 
> > > Could you try the following patch based on RMK's earlier patch
> > > without the strongly ordered patch and see if that makes any
> > > difference?
> > 
> > Maybe it lasted a few seconds longer, or maybe it was luck, but in any
> > case it eventually went into an IRQ -33 loop even with this patch.
> 
> could you try Lauri's patch posted here:
> 
> http://marc.info/?l=linux-omap&m=122407150608770&w=2
> 
> without the strongly-ordered memory patches?

I would like TI or ARM to confirm if my assumption is correct that the IRQ
will be re-raised if it gets corrupted in this way (resulting in a spurious
irq -33, which seems to be the most common one ;). If not, then this patch
will cause us to drop interrupts which is not good. Of course it is still
better than ending in a permanent loop, but only marginally.

I also tried the SO patch, result wasn't able to boot up at all (sorry, don't
have the log to show where it died).

/lauri

Re: omapfb: help from userspace

[Nathan Monson]

On Wed, Oct 15, 2008 at 4:57 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Hi Nathan,
>
> could you try Lauri's patch posted here:
> http://marc.info/?l=linux-omap&m=122407150608770&w=2
> without the strongly-ordered memory patches?

This patch works for my DSP test case.

I receive 10-20 kernel messages per second as follows:
irq -33, desc: c0414580, depth: 0, count: 0, unhandled: 0

However, DSP Bridge continues to function normally without errors.

- Nathan

Re: omapfb: help from userspace

[Tony Lindgren]

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

* Nathan Monson <nmonson@gmail.com> [081016 07:52]:
> On Wed, Oct 15, 2008 at 4:57 AM, Paul Walmsley <paul@pwsan.com> wrote:
> > Hi Nathan,
> >
> > could you try Lauri's patch posted here:
> > http://marc.info/?l=linux-omap&m=122407150608770&w=2
> > without the strongly-ordered memory patches?
> 
> This patch works for my DSP test case.
> 
> I receive 10-20 kernel messages per second as follows:
> irq -33, desc: c0414580, depth: 0, count: 0, unhandled: 0
> 
> However, DSP Bridge continues to function normally without errors.

Here's my patch to attempt to fix this issue, could you all give it a
try please? So far it has worked for me, I'd like to hear what Nathan's
dsp test case does!

Basically it's modified patch from Russell, except reading back the
revision register is not enough. Looks like we need to read back the
same register we're writing.

I guess the L3/L4 is finer grained than ARM memory attributes?

Regards,

Tony

[-- Attachment #2: write-posting.patch --]
[-- Type: text/x-diff, Size: 1731 bytes --]

>From e708817b51069acf8a9bce20713243741981bd43 Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Thu, 16 Oct 2008 10:52:51 -0700
Subject: [PATCH] ARM: OMAP: Read back the interrupt registers after write to ensure posting

On 34xx interrupts would occasionally get stuck in spurious interrupt
loop. Spurious interrupts are triggered on 34xx if the interrupt mask or
priority registers are changed while the interrupt is asserted, see
"10.5.4 MPU INTC Spurious Interrupt Handling" in the 34xx TRM.

TI's suggestion was to map L3 and L4 as strongly ordered, while Russell's
suggestion was to read back the revision register to ensure posting:

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg02904.html

However, looks like reading back INTC_REVISION is not enough with L3 and L4
busses. We need to read back the same register as we're writing to ensure
it gets posted. See also "7.3 Memory attributes" in Cortex-A8 TRM.

Signed-off-by: Tony Lindgren <tony@atomide.com>

diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 4ffb4f1..facf886 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -64,6 +64,7 @@ static u32 intc_bank_read_reg(struct omap_irq_bank *bank, u16 reg)
 static void omap_ack_irq(unsigned int irq)
 {
 	intc_bank_write_reg(0x1, &irq_banks[0], INTC_CONTROL);
+	intc_bank_read_reg(&irq_banks[0], INTC_CONTROL);
 }
 
 static void omap_mask_irq(unsigned int irq)
@@ -73,6 +74,7 @@ static void omap_mask_irq(unsigned int irq)
 	irq &= (IRQ_BITS_PER_REG - 1);
 
 	intc_bank_write_reg(1 << irq, &irq_banks[0], INTC_MIR_SET0 + offset);
+	intc_bank_read_reg(&irq_banks[0], INTC_MIR_SET0 + offset);
 }
 
 static void omap_unmask_irq(unsigned int irq)

Re: omapfb: help from userspace

[Nathan Monson]

On Thu, Oct 16, 2008 at 1:57 PM, Tony Lindgren <tony@atomide.com> wrote:
> Here's my patch to attempt to fix this issue, could you all give it a
> try please? So far it has worked for me, I'd like to hear what Nathan's
> dsp test case does!
>
> Basically it's modified patch from Russell, except reading back the
> revision register is not enough. Looks like we need to read back the
> same register we're writing.

I applied this patch and it didn't help.  I crashed once on boot-up
and then again within a couple of calls to the DSP Bridge 'ping' test,
both times with the irq -33.

Is there anything else I should have beside this patch?  I applied it
on latest linux-omap git.

- Nathan

Re: omapfb: help from userspace

[Tony Lindgren]

* Nathan Monson <nmonson@gmail.com> [081016 15:08]:
> On Thu, Oct 16, 2008 at 1:57 PM, Tony Lindgren <tony@atomide.com> wrote:
> > Here's my patch to attempt to fix this issue, could you all give it a
> > try please? So far it has worked for me, I'd like to hear what Nathan's
> > dsp test case does!
> >
> > Basically it's modified patch from Russell, except reading back the
> > revision register is not enough. Looks like we need to read back the
> > same register we're writing.
> 
> I applied this patch and it didn't help.  I crashed once on boot-up
> and then again within a couple of calls to the DSP Bridge 'ping' test,
> both times with the irq -33.
> 
> Is there anything else I should have beside this patch?  I applied it
> on latest linux-omap git.

No.. And I also got some -33 errors here too :( So back to the drawing
board.

Could you post step by step instructions on how you can reproduce your
problem?

Tony

Re: omapfb: help from userspace

[Nathan Monson]

On Thu, Oct 16, 2008 at 3:10 PM, Tony Lindgren <tony@atomide.com> wrote:
> No.. And I also got some -33 errors here too :( So back to the drawing
> board.
>
> Could you post step by step instructions on how you can reproduce your
> problem?

I'm running the DSP Bridge patches in my kernel.  I got them set up
using this guide:
http://elinux.org/BeagleBoard/DSP_Howto

That guide also points to some sample binaries, including 'ping.out',
which is a simple round-trip test between the DSP and ARM.

To reproduce, I do this:
1) Boot kernel with mem=122m
2) modprobe bridgedriver phys_mempool_base=0x87a00000
phys_mempool_size=0x600000
base_img=/home/root/dsp/ddspbase_tiomap3430.dof64P
3) while true; do ./ping.out; done

This ALWAYS reproduces within seconds.

If the DSP patches are too much work to get set up, I've also heard
that people can reproduce this issue by playing movie files with
mplayer on Angstrom.

- Nathan

Re: omapfb: help from userspace

[Tony Lindgren]

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

* Nathan Monson <nmonson@gmail.com> [081016 15:15]:
> On Thu, Oct 16, 2008 at 3:10 PM, Tony Lindgren <tony@atomide.com> wrote:
> > No.. And I also got some -33 errors here too :( So back to the drawing
> > board.
> >
> > Could you post step by step instructions on how you can reproduce your
> > problem?
> 
> I'm running the DSP Bridge patches in my kernel.  I got them set up
> using this guide:
> http://elinux.org/BeagleBoard/DSP_Howto
> 
> That guide also points to some sample binaries, including 'ping.out',
> which is a simple round-trip test between the DSP and ARM.
> 
> To reproduce, I do this:
> 1) Boot kernel with mem=122m
> 2) modprobe bridgedriver phys_mempool_base=0x87a00000
> phys_mempool_size=0x600000
> base_img=/home/root/dsp/ddspbase_tiomap3430.dof64P
> 3) while true; do ./ping.out; done
> 
> This ALWAYS reproduces within seconds.
> 
> If the DSP patches are too much work to get set up, I've also heard
> that people can reproduce this issue by playing movie files with
> mplayer on Angstrom.

Thanks, I'll set that up when I get a chance.

Meanwhile, here's one more patch for you to test if you have a chance.
Maybe also do a power off for few seconds before booting with this
just in case.

If this still does not help, then we'll use the strongly ordered patch
for now, and tag v2.6.27-omap1 tomorrow. Then we can continue debugging
the issue until we fully understand it.

Tony

[-- Attachment #2: intc-write-readback.patch --]
[-- Type: text/x-diff, Size: 347 bytes --]

--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -53,6 +53,7 @@ static struct omap_irq_bank {
 static void intc_bank_write_reg(u32 val, struct omap_irq_bank *bank, u16 reg)
 {
 	__raw_writel(val, bank->base_reg + reg);
+	__raw_readl(bank->base_reg + reg);
 }
 
 static u32 intc_bank_read_reg(struct omap_irq_bank *bank, u16 reg)

Re: omapfb: help from userspace

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [081016 15:53]:
> * Nathan Monson <nmonson@gmail.com> [081016 15:15]:
> > On Thu, Oct 16, 2008 at 3:10 PM, Tony Lindgren <tony@atomide.com> wrote:
> > > No.. And I also got some -33 errors here too :( So back to the drawing
> > > board.
> > >
> > > Could you post step by step instructions on how you can reproduce your
> > > problem?
> > 
> > I'm running the DSP Bridge patches in my kernel.  I got them set up
> > using this guide:
> > http://elinux.org/BeagleBoard/DSP_Howto
> > 
> > That guide also points to some sample binaries, including 'ping.out',
> > which is a simple round-trip test between the DSP and ARM.
> > 
> > To reproduce, I do this:
> > 1) Boot kernel with mem=122m
> > 2) modprobe bridgedriver phys_mempool_base=0x87a00000
> > phys_mempool_size=0x600000
> > base_img=/home/root/dsp/ddspbase_tiomap3430.dof64P
> > 3) while true; do ./ping.out; done
> > 
> > This ALWAYS reproduces within seconds.
> > 
> > If the DSP patches are too much work to get set up, I've also heard
> > that people can reproduce this issue by playing movie files with
> > mplayer on Angstrom.
> 
> Thanks, I'll set that up when I get a chance.
> 
> Meanwhile, here's one more patch for you to test if you have a chance.
> Maybe also do a power off for few seconds before booting with this
> just in case.
> 
> If this still does not help, then we'll use the strongly ordered patch
> for now, and tag v2.6.27-omap1 tomorrow. Then we can continue debugging
> the issue until we fully understand it.

Well with this one I eventually got one -33 error, but not a series of
them.

> Tony

> --- a/arch/arm/mach-omap2/irq.c
> +++ b/arch/arm/mach-omap2/irq.c
> @@ -53,6 +53,7 @@ static struct omap_irq_bank {
>  static void intc_bank_write_reg(u32 val, struct omap_irq_bank *bank, u16 reg)
>  {
>  	__raw_writel(val, bank->base_reg + reg);
> +	__raw_readl(bank->base_reg + reg);
>  }
>  
>  static u32 intc_bank_read_reg(struct omap_irq_bank *bank, u16 reg)

Re: omapfb: help from userspace

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [081016 15:59]:
> * Tony Lindgren <tony@atomide.com> [081016 15:53]:
> > * Nathan Monson <nmonson@gmail.com> [081016 15:15]:
> > > On Thu, Oct 16, 2008 at 3:10 PM, Tony Lindgren <tony@atomide.com> wrote:
> > > > No.. And I also got some -33 errors here too :( So back to the drawing
> > > > board.
> > > >
> > > > Could you post step by step instructions on how you can reproduce your
> > > > problem?
> > > 
> > > I'm running the DSP Bridge patches in my kernel.  I got them set up
> > > using this guide:
> > > http://elinux.org/BeagleBoard/DSP_Howto
> > > 
> > > That guide also points to some sample binaries, including 'ping.out',
> > > which is a simple round-trip test between the DSP and ARM.
> > > 
> > > To reproduce, I do this:
> > > 1) Boot kernel with mem=122m
> > > 2) modprobe bridgedriver phys_mempool_base=0x87a00000
> > > phys_mempool_size=0x600000
> > > base_img=/home/root/dsp/ddspbase_tiomap3430.dof64P
> > > 3) while true; do ./ping.out; done
> > > 
> > > This ALWAYS reproduces within seconds.
> > > 
> > > If the DSP patches are too much work to get set up, I've also heard
> > > that people can reproduce this issue by playing movie files with
> > > mplayer on Angstrom.
> > 
> > Thanks, I'll set that up when I get a chance.
> > 
> > Meanwhile, here's one more patch for you to test if you have a chance.
> > Maybe also do a power off for few seconds before booting with this
> > just in case.
> > 
> > If this still does not help, then we'll use the strongly ordered patch
> > for now, and tag v2.6.27-omap1 tomorrow. Then we can continue debugging
> > the issue until we fully understand it.
> 
> Well with this one I eventually got one -33 error, but not a series of
> them.

This is with Lauri's patch and this patch applied.

> > Tony
> 
> > --- a/arch/arm/mach-omap2/irq.c
> > +++ b/arch/arm/mach-omap2/irq.c
> > @@ -53,6 +53,7 @@ static struct omap_irq_bank {
> >  static void intc_bank_write_reg(u32 val, struct omap_irq_bank *bank, u16 reg)
> >  {
> >  	__raw_writel(val, bank->base_reg + reg);
> > +	__raw_readl(bank->base_reg + reg);
> >  }
> >  
> >  static u32 intc_bank_read_reg(struct omap_irq_bank *bank, u16 reg)
> 

Re: omapfb: help from userspace

[Nathan Monson]

On Thu, Oct 16, 2008 at 4:02 PM, Tony Lindgren <tony@atomide.com> wrote:
> Well with this one I eventually got one -33 error, but not a series of
> them.
>
> This is with Lauri's patch and this patch applied.

With this patch alone, or with this patch plus your earlier patch, I
still get a dead kernel printing an endless series of the -33 errors.

With Lauri's patch alone, I can run the DSP all day.  I still get a
fairly steady stream of -33s errors, but the kernel never gets stuck
in a loop and the DSP continues working.

With the S-O patch, everything works.  No errors.

- Nathan

Re: omapfb: help from userspace

[Tony Lindgren]

* Nathan Monson <nmonson@gmail.com> [081016 16:20]:
> On Thu, Oct 16, 2008 at 4:02 PM, Tony Lindgren <tony@atomide.com> wrote:
> > Well with this one I eventually got one -33 error, but not a series of
> > them.
> >
> > This is with Lauri's patch and this patch applied.
> 
> With this patch alone, or with this patch plus your earlier patch, I
> still get a dead kernel printing an endless series of the -33 errors.
> 
> With Lauri's patch alone, I can run the DSP all day.  I still get a
> fairly steady stream of -33s errors, but the kernel never gets stuck
> in a loop and the DSP continues working.
> 
> With the S-O patch, everything works.  No errors.

OK, time to push the SO patch, can you reply with your Signed-off-by
for that?

Thanks,

Tony

Re: omapfb: help from userspace

[Tony Lindgren]

* Nathan Monson <nmonson@gmail.com> [081008 13:22]:
> On Wed, Oct 8, 2008 at 11:36 AM, Nathan Monson <nmonson@gmail.com> wrote:
> > "Felipe Contreras" <felipe.contreras@gmail.com> writes:
> >> irq -33, desc: c0335cf8, depth: 0, count: 0, unhandled: 0
> >
> > On the BeagleBoard list, Pratheesh Gangadhar said that mapping I/O
> > regions as Strongly Ordered suppresses this problem:
> > http://groups.google.com/group/beagleboard/browse_thread/thread/23e1c95b4bfb09b5/70d12dca569ca503?show_docid=70d12dca569ca503
> 
> Pratheesh helped me make a patch against the latest linux-omap git to
> try this.
> 
> With this patch, my IRQ -33 problems with the DSP have disappeared.
> Before, I would end up in IRQ -33 loop after 10 invocations of the DSP
> Bridge 'ping.out' utility.  I just finished running it 50,000 times
> without error.
> 
> As stated before, this patch is just a workaround for testing
> purposes, not a fix.  Who knows what performance side effects it
> has...

OK, pushed this for now until we know if there's a better solution.

Tony

> 
> ---
> diff --git a/arch/arm/include/asm/mach/map.h b/arch/arm/include/asm/mach/map.h
> index 9eb936e..5cb4f5f 100644
> --- a/arch/arm/include/asm/mach/map.h
> +++ b/arch/arm/include/asm/mach/map.h
> @@ -25,6 +25,7 @@ struct map_desc {
>  #define MT_HIGH_VECTORS		8
>  #define MT_MEMORY		9
>  #define MT_ROM			10
> +#define MT_MEMORY_SO		11
> 
>  #define MT_NONSHARED_DEVICE	MT_DEVICE_NONSHARED
>  #define MT_IXP2000_DEVICE	MT_DEVICE_IXP2000
> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
> index adbe21f..c11c0e8 100644
> --- a/arch/arm/mach-omap2/io.c
> +++ b/arch/arm/mach-omap2/io.c
> @@ -119,13 +119,13 @@ static struct map_desc omap34xx_io_desc[] __initdata = {
>  		.virtual	= L3_34XX_VIRT,
>  		.pfn		= __phys_to_pfn(L3_34XX_PHYS),
>  		.length		= L3_34XX_SIZE,
> -		.type		= MT_DEVICE
> +		.type		= MT_MEMORY_SO
>  	},
>  	{
>  		.virtual	= L4_34XX_VIRT,
>  		.pfn		= __phys_to_pfn(L4_34XX_PHYS),
>  		.length		= L4_34XX_SIZE,
> -		.type		= MT_DEVICE
> +		.type		= MT_MEMORY_SO
>  	},
>  	{
>  		.virtual	= L4_WK_34XX_VIRT,
> @@ -137,19 +137,19 @@ static struct map_desc omap34xx_io_desc[] __initdata = {
>  		.virtual	= OMAP34XX_GPMC_VIRT,
>  		.pfn		= __phys_to_pfn(OMAP34XX_GPMC_PHYS),
>  		.length		= OMAP34XX_GPMC_SIZE,
> -		.type		= MT_DEVICE
> +		.type		= MT_MEMORY_SO
>  	},
>  	{
>  		.virtual	= OMAP343X_SMS_VIRT,
>  		.pfn		= __phys_to_pfn(OMAP343X_SMS_PHYS),
>  		.length		= OMAP343X_SMS_SIZE,
> -		.type		= MT_DEVICE
> +		.type		= MT_MEMORY_SO
>  	},
>  	{
>  		.virtual	= OMAP343X_SDRC_VIRT,
>  		.pfn		= __phys_to_pfn(OMAP343X_SDRC_PHYS),
>  		.length		= OMAP343X_SDRC_SIZE,
> -		.type		= MT_DEVICE
> +		.type		= MT_MEMORY_SO
>  	},
>  	{
>  		.virtual	= L4_PER_34XX_VIRT,
> @@ -161,7 +161,7 @@ static struct map_desc omap34xx_io_desc[] __initdata = {
>  		.virtual	= L4_EMU_34XX_VIRT,
>  		.pfn		= __phys_to_pfn(L4_EMU_34XX_PHYS),
>  		.length		= L4_EMU_34XX_SIZE,
> -		.type		= MT_DEVICE
> +		.type		= MT_MEMORY_SO
>  	},
>  };
>  #endif
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index a713e40..d5f25ad 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -245,6 +245,10 @@ static struct mem_type mem_types[] = {
>  		.prot_sect = PMD_TYPE_SECT,
>  		.domain    = DOMAIN_KERNEL,
>  	},
> +	[MT_MEMORY_SO] = {
> +		.prot_sect = PMD_TYPE_SECT | PMD_SECT_AP_WRITE | PMD_SECT_UNCACHED,
> +		.domain    = DOMAIN_KERNEL,
> +	},
>  };
> 
>  const struct mem_type *get_mem_type(unsigned int type)
> --
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html