[Xorg-driver-geode] Who can explain the diff between Xserver-1.6.4 version and >1.7 version about the ExaGetPixmapAddress?

[Cui, Hunk]

Hi, Maarten,

	In our xf86-video-geode driver, all of memories are allocated by GeodeAllocOffscreen, the exa offscreen memory is part of the memorySize. And the rotation data is not in memorySize, it is allocated after memorySize.

	About the GeodeAllocOffscreen, it is Data structure tables, why you said "allocate exa offscreen memory out of a private pool"? What the diff between GeodeAllocOffscreen and exaOffscreen? In exaOffscreenInit, the EXA offscreen base and size are loaded into server, so the exa should recognize it as offscreen memory, furthermore, the ratate_memory(2MB) is not included in EXA offscreen space. 
	That "pPixData - pExaScr->info->memoryBase = pExaScr->info->memorySize" should right.

	If you confuse in this explain, I can provide a chart about the memory allocate.

Thanks,
Hunk Cui



-----Original Message-----
From: Maarten Maathuis [mailto:madman2003 at gmail.com] 
Sent: Sunday, June 13, 2010 6:08 PM
To: Cui, Hunk
Cc: xorg-devel at lists.x.org; xorg-driver-geode at lists.x.org; Michel Dänzer
Subject: Re: Who can explain the diff between Xserver-1.6.4 version and >1.7 version about the ExaGetPixmapAddress?

On Sun, Jun 13, 2010 at 10:18 AM, Cui, Hunk <Hunk.Cui at amd.com> wrote:
> Hi, Maarten,
>
>        After I debug the GeodeAllocOffscreen() operation (it is an internal function), I found it have been replaced exaOffscreenAlloc() with GeodeAllocOffscreen().
>
>        About the change, you can see: http://cgit.freedesktop.org/xorg/driver/xf86-video-geode/commit/?id=d681a844e448712a9a419d2a4dca81930d39a80a
>
>        A wholesale update to Randr 1.2 for LX accompanied by massive
> cleanup since 08/07/2008, include exaOffscreenAlloc(), because no longer to maintain the gx_video. So you can find the exaOffscreenAlloc on Geode-gx. And now, only need to maintain Geode-LX, I believe the GeodeAllocOffscreen() have been update to a strong effort. It have been allocated the Geode-LX memory, include Compression buffer, TryHWCursor, exaBfSz, EXA offscreen space, a shadow buffer, a video overlay. (you can see in lx_memory.c -> LXInitOffscreen).
>
>        My issue is RandR-unable to rotate. (I have been described the phenomenon in: https://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-geode/+bug/377929/comments/12 ).
>
>        I have been debug the memory allocation method in LXInitOffscreen. After allocate the EXA offscreen space (http://cgit.freedesktop.org/xorg/driver/xf86-video-geode/tree/src/lx_memory.c#n261 )
> The crtc shadow frame buffer (Rotate_memory) allocate:
> ________
>        size -= pScrni->virtualX *
>        (pScrni->virtualY * (pScrni->bitsPerPixel >> 3));
> ________about 2MB size
>
> The video overlay size is 2MB size.
> Then the memorySize = offscreenBase + EXA offscreen Space size
>
> Steps:
>        1). In exaOffscreenInit (Xserver: exa_offscreen.c: 677), the offscreenAreas struct will be allocated. When do rotate action.
>        2). The client program will call the xf86CrtcRotate (In xf86Rotate.c), then it will allocate the Rotate_memory in shadow frame buffer (less than 2MB).
>        3). Xf86RotatePrepare -> lx_crtc_shadow_create -> GetScratchPixmapHeader -> exaModifyPixmapHeader_classic, the rotate_memory_base will be loaded into pPixData, the value is equal to memorySize+memoryBase, So "pPixData - pExaScr->info->memoryBase = pExaScr->info->memorySize", but now in Xserver, it only have "<". The judge is not come into existence. The fb_ptr address value is 0x0.

You allocate exa offscreen memory out of a private pool with
GeodeAllocOffscreen, only that memory is considered usable for
acceleration by exa. Your proposed change to <= is taping over the
real issue. GeodeAllocOffscreen seems to give you memory directly
after exa's offscreen memory, but that doesn't mean exa should
recognize it as offscreen memory. Your change would hack exa to accept
one extra byte as valid, but shouldn't your whole 2 MB region be
inside? If the whole 2 MB was inside < or <= wouldn't even matter.

I've cc'ed someone else who also knows a lot about exa, but i still
stand by my judgement that you need to use exaOffscreenAlloc for the
rotation data, and not some private memory exa knows nothing about.

>
>        I think the GeodeAllocOffscreen() memory allocate action is right. Please take a look, I have not find another reason. So I suggest to add "=".
>
>        Looking forward to your reply.
>
> Thanks,
> Hunk Cui
>