[Intel-gfx] xf86-video-intel: Commit 08ebde4715b87867184d42b60762cd774e151f5c breaks kernel memory manager allocation

[Wu Fengguang]

On Wed, Apr 22, 2009 at 08:40:05AM +0800, Zhenyu Wang wrote:
> On 2009.04.21 20:29:26 +0800, Magnus Kessler wrote:
> > Hi Zhenyu,
> > 
> > your commit 08ebde4715b87867184d42b60762cd774e151f5c to the intel video driver 
> > breaks kernel memory allocation for me on a 64 bit platform. Reverting the 
> > patch gets me back to a working driver. The relevant output from xorg.log is:
> 
> sorry, I didn't test Fengguang's patch on 64 bit system, on which his patch aims
> to fix a compiler warning...
> 
> Fengguang, have you tested this on x86_64 with recent kernel? 
> 
> Magnus, any possible to ErrorF some debug message on why memory manager allocation
> failed?

I used the following patch to debug this issue. Got these outputs:

        videoRam =  ~0UL / KB(1);
OK      wfg: offset=0 end=0 next-offset=18446744073709550592

        videoRam =  INT_MAX / KB(1);
FAIL    wfg: offset=0 end=2147483648 next-offset=2147482624
(failed because end > next-offset, here is a tricky round-up issue)

        videoRam = 0;
        size = ULONG_MAX;
OK      wfg: offset=0 end=0 next-offset=18446744073709551615

More tests which fake a smaller mmsize:

        original ~0UL
        mmsize -= 9999;
OK      wfg: offset=0 end=18446744073709543424 next-offset=18446744073709550592

        patched ULONG_MAX
        mmsize -= 9999;
OK      wfg: offset=0 end=18446744073709543424 next-offset=18446744073709551615

--- a/src/i830_memory.c
+++ b/src/i830_memory.c
@@ -446,6 +449,7 @@ i830_allocator_init(ScrnInfoPtr pScrn, unsigned long offset, unsigned long size)
 	    mmsize -= MB(6) + ROUND_TO_PAGE(FBC_LL_SIZE + FBC_LL_PAD);
 	/* Can't do GEM on stolen memory */
 	mmsize -= pI830->stolen_size;
+	mmsize -= 9999;
 
 	/* Create the aperture allocation */
 	pI830->memory_manager =
@@ -660,6 +664,8 @@ i830_allocate_aperture(ScrnInfoPtr pScrn, const char *name,
 	mem->end = mem->offset + size;
 	if (flags & ALIGN_BOTH_ENDS)
 	    mem->end = ROUND_TO(mem->end, alignment);
+	fprintf(stderr, "wfg: offset=%lu end=%lu next-offset=%lu\n",
+			mem->offset, mem->end, scan->next->offset);
 	if (mem->end <= scan->next->offset)
 	    break;
     }


In the process I discovered it would better to remove the @offset
parameter from i830_allocator_init() as in the following patch.

Thanks,
Fengguang
---
Remove the @offset param from i830_allocator_init()

@offset is a redundant parameter.

Signed-off-by: Wu Fengguang <fengguang.wu at intel.com>
---
 src/i830.h        |    3 +--
 src/i830_driver.c |    2 +-
 src/i830_memory.c |    6 +++---
 3 files changed, 5 insertions(+), 6 deletions(-)

--- xf86-video-intel.orig/src/i830.h
+++ xf86-video-intel/src/i830.h
@@ -692,8 +692,7 @@ extern void I830SetupForSolidFill(ScrnIn
 extern void I830SubsequentSolidFillRect(ScrnInfoPtr pScrn, int x, int y,
 					int w, int h);
 
-Bool i830_allocator_init(ScrnInfoPtr pScrn, unsigned long offset,
-			 unsigned long size);
+Bool i830_allocator_init(ScrnInfoPtr pScrn, unsigned long size);
 void i830_allocator_fini(ScrnInfoPtr pScrn);
 i830_memory * i830_allocate_memory(ScrnInfoPtr pScrn, const char *name,
 				   unsigned long size, unsigned long pitch,
--- xf86-video-intel.orig/src/i830_driver.c
+++ xf86-video-intel/src/i830_driver.c
@@ -2319,7 +2319,7 @@ i830_memory_init(ScrnInfoPtr pScrn)
 
     tiled = i830_tiled_width(pI830, &pScrn->displayWidth, pI830->cpp);
     /* Set up our video memory allocator for the chosen videoRam */
-    if (!i830_allocator_init(pScrn, 0, pScrn->videoRam * KB(1))) {
+    if (!i830_allocator_init(pScrn, pScrn->videoRam * KB(1))) {
 	xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
 		"Couldn't initialize video memory allocator\n");
 	PreInitCleanup(pScrn);
--- xf86-video-intel.orig/src/i830_memory.c
+++ xf86-video-intel/src/i830_memory.c
@@ -364,7 +364,7 @@ i830_reset_allocations(ScrnInfoPtr pScrn
  * addresses to reference.
  */
 Bool
-i830_allocator_init(ScrnInfoPtr pScrn, unsigned long offset, unsigned long size)
+i830_allocator_init(ScrnInfoPtr pScrn, unsigned long size)
 {
     I830Ptr pI830 = I830PTR(pScrn);
     i830_memory *start, *end;
@@ -398,12 +398,12 @@ i830_allocator_init(ScrnInfoPtr pScrn, u
 	    size = ULONG_MAX;
 
     start->key = -1;
-    start->offset = offset;
+    start->offset = 0;
     start->end = start->offset;
     start->size = 0;
     start->next = end;
     end->key = -1;
-    end->offset = offset + size;
+    end->offset = size;
     end->end = end->offset;
     end->size = 0;
     end->prev = start;