Mainline OMAP3 breakage (and other OMAP?)

* Russell King - ARM Linux [off-list ref] [101202 13:04]:

Great. Adding fbdev and Tomi to Cc.

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

On Thu, Dec 02, 2010 at 01:58:38PM -0800, Tony Lindgren wrote:

* Russell King - ARM Linux [off-list ref] [101202 13:04]:

quoted

This has been around since October:

drivers/video/omap2/vram.c: In function ■omap_vram_reserve_sdram_memblock■:
drivers/video/omap2/vram.c:573: error: ■MEMBLOCK_REAL_LIMIT■ undeclared (first use in this function)
drivers/video/omap2/vram.c:573: error: (Each undeclared identifier is reported only once
drivers/video/omap2/vram.c:573: error: for each function it appears in.)

This requires a trivial one-liner compile fix:

diff --git a/drivers/video/omap2/vram.c b/drivers/video/omap2/vram.c
index fed2a72..a8973f0 100644
--- a/drivers/video/omap2/vram.c
+++ b/drivers/video/omap2/vram.c

@@ -570,7 +570,7 @@ void __init omap_vram_reserve_sdram_memblock(void)
 			return;
 		}
 	} else {
-		paddr = memblock_alloc_base(size, PAGE_SIZE, MEMBLOCK_REAL_LIMIT);
+		paddr = memblock_alloc(size, PAGE_SIZE);
 	}
 
 	omap_vram_add_region(paddr, size);

which restores the old behaviour before the X86 memblock changes went
in.  Yes, there may be other changes due to the ioremap stuff, but
that's really no excuse for not fixing the compile error itself.

Great. Adding fbdev and Tomi to Cc.

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

http://marc.info/?l=linux-omap&w=2&r=1&s=MEMBLOCK_REAL_LIMIT%20vram&q=b

There have been patches posted throughout November to fix this, but
the problem is they're not making it to mainline.  It needs chasing
until someone does the right thing and sends one variant of the above
patch, rather than just leaving it until the ioremap fixes hit
mainline during the next merge window.

* Russell King - ARM Linux [off-list ref] [101202 14:06]:

On Thu, Dec 02, 2010 at 01:58:38PM -0800, Tony Lindgren wrote:

quoted

* Russell King - ARM Linux [off-list ref] [101202 13:04]:

quoted

This has been around since October:

drivers/video/omap2/vram.c: In function ■omap_vram_reserve_sdram_memblock■:
drivers/video/omap2/vram.c:573: error: ■MEMBLOCK_REAL_LIMIT■ undeclared (first use in this function)
drivers/video/omap2/vram.c:573: error: (Each undeclared identifier is reported only once
drivers/video/omap2/vram.c:573: error: for each function it appears in.)

This requires a trivial one-liner compile fix:

diff --git a/drivers/video/omap2/vram.c b/drivers/video/omap2/vram.c
index fed2a72..a8973f0 100644
--- a/drivers/video/omap2/vram.c
+++ b/drivers/video/omap2/vram.c

@@ -570,7 +570,7 @@ void __init omap_vram_reserve_sdram_memblock(void)
 			return;
 		}
 	} else {
-		paddr = memblock_alloc_base(size, PAGE_SIZE, MEMBLOCK_REAL_LIMIT);
+		paddr = memblock_alloc(size, PAGE_SIZE);
 	}
 
 	omap_vram_add_region(paddr, size);

which restores the old behaviour before the X86 memblock changes went
in.  Yes, there may be other changes due to the ioremap stuff, but
that's really no excuse for not fixing the compile error itself.

Great. Adding fbdev and Tomi to Cc.

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

http://marc.info/?l=linux-omap&w=2&r=1&s=MEMBLOCK_REAL_LIMIT%20vram&q=b

There have been patches posted throughout November to fix this, but
the problem is they're not making it to mainline.  It needs chasing
until someone does the right thing and sends one variant of the above
patch, rather than just leaving it until the ioremap fixes hit
mainline during the next merge window.

Yes this should go in during the -rc for sure.

I suggest you merge this but let's wait a bit and check if Tomi
already has a similar fix queued for the -rc series.

Regards,

Tony

On Thu, Dec 02, 2010 at 02:32:08PM -0800, Tony Lindgren wrote:

* Russell King - ARM Linux [off-list ref] [101202 14:06]:

quoted

On Thu, Dec 02, 2010 at 01:58:38PM -0800, Tony Lindgren wrote:

quoted

* Russell King - ARM Linux [off-list ref] [101202 13:04]:

quoted

This has been around since October:

drivers/video/omap2/vram.c: In function ???omap_vram_reserve_sdram_memblock???:
drivers/video/omap2/vram.c:573: error: ???MEMBLOCK_REAL_LIMIT??? undeclared (first use in this function)
drivers/video/omap2/vram.c:573: error: (Each undeclared identifier is reported only once
drivers/video/omap2/vram.c:573: error: for each function it appears in.)

This requires a trivial one-liner compile fix:

diff --git a/drivers/video/omap2/vram.c b/drivers/video/omap2/vram.c
index fed2a72..a8973f0 100644
--- a/drivers/video/omap2/vram.c
+++ b/drivers/video/omap2/vram.c

@@ -570,7 +570,7 @@ void __init omap_vram_reserve_sdram_memblock(void)
 			return;
 		}
 	} else {
-		paddr = memblock_alloc_base(size, PAGE_SIZE, MEMBLOCK_REAL_LIMIT);
+		paddr = memblock_alloc(size, PAGE_SIZE);
 	}
 
 	omap_vram_add_region(paddr, size);

which restores the old behaviour before the X86 memblock changes went
in.  Yes, there may be other changes due to the ioremap stuff, but
that's really no excuse for not fixing the compile error itself.

Great. Adding fbdev and Tomi to Cc.

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

http://marc.info/?l=linux-omap&w=2&r=1&s=MEMBLOCK_REAL_LIMIT%20vram&q=b

There have been patches posted throughout November to fix this, but
the problem is they're not making it to mainline.  It needs chasing
until someone does the right thing and sends one variant of the above
patch, rather than just leaving it until the ioremap fixes hit
mainline during the next merge window.

Yes this should go in during the -rc for sure.

I suggest you merge this but let's wait a bit and check if Tomi
already has a similar fix queued for the -rc series.

This has been fixed since -rc2.

On Fri, Dec 03, 2010 at 12:09:55PM +0900, Paul Mundt wrote:

On Thu, Dec 02, 2010 at 02:32:08PM -0800, Tony Lindgren wrote:

quoted

* Russell King - ARM Linux [off-list ref] [101202 14:06]:

quoted

On Thu, Dec 02, 2010 at 01:58:38PM -0800, Tony Lindgren wrote:

quoted

* Russell King - ARM Linux [off-list ref] [101202 13:04]:

quoted

This has been around since October:

drivers/video/omap2/vram.c: In function ???omap_vram_reserve_sdram_memblock???:
drivers/video/omap2/vram.c:573: error: ???MEMBLOCK_REAL_LIMIT??? undeclared (first use in this function)
drivers/video/omap2/vram.c:573: error: (Each undeclared identifier is reported only once
drivers/video/omap2/vram.c:573: error: for each function it appears in.)

This requires a trivial one-liner compile fix:

diff --git a/drivers/video/omap2/vram.c b/drivers/video/omap2/vram.c
index fed2a72..a8973f0 100644
--- a/drivers/video/omap2/vram.c
+++ b/drivers/video/omap2/vram.c

@@ -570,7 +570,7 @@ void __init omap_vram_reserve_sdram_memblock(void)
 			return;
 		}
 	} else {
-		paddr = memblock_alloc_base(size, PAGE_SIZE, MEMBLOCK_REAL_LIMIT);
+		paddr = memblock_alloc(size, PAGE_SIZE);
 	}
 
 	omap_vram_add_region(paddr, size);

which restores the old behaviour before the X86 memblock changes went
in.  Yes, there may be other changes due to the ioremap stuff, but
that's really no excuse for not fixing the compile error itself.

Great. Adding fbdev and Tomi to Cc.

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

http://marc.info/?l=linux-omap&w=2&r=1&s=MEMBLOCK_REAL_LIMIT%20vram&q=b

There have been patches posted throughout November to fix this, but
the problem is they're not making it to mainline.  It needs chasing
until someone does the right thing and sends one variant of the above
patch, rather than just leaving it until the ioremap fixes hit
mainline during the next merge window.

Yes this should go in during the -rc for sure.

I suggest you merge this but let's wait a bit and check if Tomi
already has a similar fix queued for the -rc series.

This has been fixed since -rc2.

So it is.  However, the ioremap fix is wrong.

        } else {
                paddr = memblock_alloc(size, PAGE_SIZE);
        }

        memblock_free(paddr, size);
        memblock_remove(paddr, size);

Didn't I say that such blocks were supposed to be aligned to a 2MB
boundary - and the size also so aligned?

http://lists.arm.linux.org.uk/lurker/message/20101011.152516.893a6088.en.html

It at least needs to be 1MB as that's the section size.

On Fri, Dec 3, 2010 at 10:41 AM, Russell King - ARM Linux
[off-list ref] wrote:

On Fri, Dec 03, 2010 at 12:09:55PM +0900, Paul Mundt wrote:

quoted

This has been fixed since -rc2.

So it is.  However, the ioremap fix is wrong.

       } else {
               paddr = memblock_alloc(size, PAGE_SIZE);
       }

       memblock_free(paddr, size);
       memblock_remove(paddr, size);

Didn't I say that such blocks were supposed to be aligned to a 2MB
boundary - and the size also so aligned?

I asked you if there was something special and I didn't receive any response:
http://article.gmane.org/gmane.linux.kernel/1048434

So, why SZ_2M?

-- 
Felipe Contreras

On Fri, Dec 03, 2010 at 02:10:23PM +0200, Felipe Contreras wrote:

On Fri, Dec 3, 2010 at 10:41 AM, Russell King - ARM Linux
[off-list ref] wrote:

quoted

On Fri, Dec 03, 2010 at 12:09:55PM +0900, Paul Mundt wrote:

quoted

This has been fixed since -rc2.

So it is.  However, the ioremap fix is wrong.

       } else {
               paddr = memblock_alloc(size, PAGE_SIZE);
       }

       memblock_free(paddr, size);
       memblock_remove(paddr, size);

Didn't I say that such blocks were supposed to be aligned to a 2MB
boundary - and the size also so aligned?

I asked you if there was something special and I didn't receive any response:
http://article.gmane.org/gmane.linux.kernel/1048434

So you thought you could ride rough-shod over my statement and ignore it?
Thank you for treating me like shit, I'll remember that for future and
treat you in a similar manner.

So, why SZ_2M?

Firstly, that's the granularity which we allocate page tables - one
Linux page table covers 2MB of memory.  We want to avoid creating page
tables for the main memory mapping as that increases TLB pressure through
the use of additional TLB entries, and more page table walks.

Plus, we never used to allow the kernel's direct memory mapping to be
mapped at anything less than section size - this restriction has since
been lifted due to OMAP SRAM problems, but I'd rather we stuck with it
to ensure that we have proper behaviour from all parts of the system.

Secondly, we don't want to end up with lots of fragmentation at the end
of the memory mapping as that'll reduce performance, not only by making
the pfn_valid() search more expensive.

Emsuring a minimum allocation size and alignment makes sure that the
regions can be coalesced together into one block, and minimises run-time
expenses.

So please, 2MB, or if you object, at the _very_ _least_ 1MB.  But
definitely not PAGE_SIZE.

On Fri, Dec 3, 2010 at 3:07 PM, Russell King - ARM Linux
[off-list ref] wrote:

On Fri, Dec 03, 2010 at 02:10:23PM +0200, Felipe Contreras wrote:

quoted

On Fri, Dec 3, 2010 at 10:41 AM, Russell King - ARM Linux
[off-list ref] wrote:

quoted

On Fri, Dec 03, 2010 at 12:09:55PM +0900, Paul Mundt wrote:

quoted

This has been fixed since -rc2.

So it is.  However, the ioremap fix is wrong.

       } else {
               paddr = memblock_alloc(size, PAGE_SIZE);
       }

       memblock_free(paddr, size);
       memblock_remove(paddr, size);

Didn't I say that such blocks were supposed to be aligned to a 2MB
boundary - and the size also so aligned?

I asked you if there was something special and I didn't receive any response:
http://article.gmane.org/gmane.linux.kernel/1048434

So you thought you could ride rough-shod over my statement and ignore it?
Thank you for treating me like shit, I'll remember that for future and
treat you in a similar manner.

No, you didn't make any statement, just provided code. I thought SZ_2M
might have just been an example for something that required that
alignment, thus my request for clarification.

quoted

So, why SZ_2M?

Firstly, that's the granularity which we allocate page tables - one
Linux page table covers 2MB of memory.  We want to avoid creating page
tables for the main memory mapping as that increases TLB pressure through
the use of additional TLB entries, and more page table walks.

Plus, we never used to allow the kernel's direct memory mapping to be
mapped at anything less than section size - this restriction has since
been lifted due to OMAP SRAM problems, but I'd rather we stuck with it
to ensure that we have proper behaviour from all parts of the system.

Secondly, we don't want to end up with lots of fragmentation at the end
of the memory mapping as that'll reduce performance, not only by making
the pfn_valid() search more expensive.

Emsuring a minimum allocation size and alignment makes sure that the
regions can be coalesced together into one block, and minimises run-time
expenses.

So please, 2MB, or if you object, at the _very_ _least_ 1MB.  But
definitely not PAGE_SIZE.

Fair enough. If nobody beats me to it I would send patches to fix
that, although perhaps it would make sense to write a helper function
that does exactly this.

Cheers.

-- 
Felipe Contreras

Hi,

On Fri, 2010-12-03 at 13:07 +0000, ext Russell King - ARM Linux wrote:

So please, 2MB, or if you object, at the _very_ _least_ 1MB.  But
definitely not PAGE_SIZE.

Here's a patch for this. Works for me on OMAP3430SDP. If the patch is
ok, I'll send a pull request to Paul.



From 6c54704730626e2683e05574b3cbba966980c956 Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen <redacted>
Date: Tue, 14 Dec 2010 14:16:59 +0200
Subject: [PATCH] OMAP: DSS: VRAM: Align start & size of vram to 2M

Align the start address and size of VRAM area to 2M as per comments from
Russell King:

quoted

So, why SZ_2M?

Firstly, that's the granularity which we allocate page tables - one
Linux page table covers 2MB of memory.  We want to avoid creating page
tables for the main memory mapping as that increases TLB pressure through
the use of additional TLB entries, and more page table walks.

Plus, we never used to allow the kernel's direct memory mapping to be
mapped at anything less than section size - this restriction has since
been lifted due to OMAP SRAM problems, but I'd rather we stuck with it
to ensure that we have proper behaviour from all parts of the system.

Secondly, we don't want to end up with lots of fragmentation at the end
of the memory mapping as that'll reduce performance, not only by making
the pfn_valid() search more expensive.

Emsuring a minimum allocation size and alignment makes sure that the
regions can be coalesced together into one block, and minimises run-time
expenses.

So please, 2MB, or if you object, at the _very_ _least_ 1MB.  But
definitely not PAGE_SIZE.

Signed-off-by: Tomi Valkeinen <redacted>
---
 drivers/video/omap2/vram.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/vram.c b/drivers/video/omap2/vram.c
index 2fd7e52..9441e2e 100644
--- a/drivers/video/omap2/vram.c
+++ b/drivers/video/omap2/vram.c

@@ -551,7 +551,7 @@ void __init omap_vram_reserve_sdram_memblock(void)
 	if (!size)
 		return;
 
-	size = PAGE_ALIGN(size);
+	size = ALIGN(size, SZ_2M);
 
 	if (paddr) {
 		if (paddr & ~PAGE_MASK) {

@@ -576,7 +576,7 @@ void __init omap_vram_reserve_sdram_memblock(void)
 			return;
 		}
 	} else {
-		paddr = memblock_alloc(size, PAGE_SIZE);
+		paddr = memblock_alloc(size, SZ_2M);
 	}
 
 	memblock_free(paddr, size);

-- 
1.7.1

* Tomi Valkeinen [off-list ref] [101214 04:23]:

quoted

So please, 2MB, or if you object, at the _very_ _least_ 1MB.  But
definitely not PAGE_SIZE.

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