Who can explain the diff between Xserver-1.6.4 version and >1.7 version about the ExaGetPixmapAddress?

Sun Jun 13 03:07:34 PDT 2010

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
>
>
>
>
>
>
> -----Original Message-----
> From: Maarten Maathuis [mailto:madman2003 at gmail.com]
> Sent: Thursday, June 10, 2010 12:42 AM
> To: Cui, Hunk
> Cc: xorg-devel at lists.x.org; Huang, FrankR; Writer, Tim; Torres, Rigo
> Subject: Re: Who can explain the diff between Xserver-1.6.4 version and >1.7 version about the ExaGetPixmapAddress?
>
> On Wed, Jun 9, 2010 at 1:00 PM, Cui, Hunk <Hunk.Cui at amd.com> wrote:
>> Hi, Maarten,
>>
>>        Thank you very much for your help, I am looking forward to your reply. :)
>
> GeodeAllocOffscreen() is an internal function, exa doesn't know this
> memory. The idea is to remove this from src/lx_memory.c:253
>
> --------
>        /* Deduct the probable size of a shadow buffer */
>        size -= pScrni->virtualX *
>        (pScrni->virtualY * (pScrni->bitsPerPixel >> 3));
> --------
>
> This will then be added to the exa pool, you can use
> exaOffscreenAlloc. See
> http://cgit.freedesktop.org/xorg/driver/xf86-video-ati/tree/src/radeon_legacy_memory.c#n50
> for appropriate arguments and usage. The offset is relative to memory
> base.
>
> The same is true for the video overlay, gx_video seems to be using
> exaOffscreenAlloc already.
>
>>
>> Thanks,
>> Hunk Cui
>>
>> -----Original Message-----
>> From: Maarten Maathuis [mailto:madman2003 at gmail.com]
>> Sent: Wednesday, June 09, 2010 6:57 PM
>> To: Cui, Hunk
>> Cc: xorg-devel at lists.x.org; Huang, FrankR; Writer, Tim; Torres, Rigo
>> Subject: Re: Who can explain the diff between Xserver-1.6.4 version and >1.7 version about the ExaGetPixmapAddress?
>>
>> On Wed, Jun 9, 2010 at 12:37 PM, Cui, Hunk <Hunk.Cui at amd.com> wrote:
>>> Hi, Maarten,
>>>
>>>        About the crtc->rotatedData (AMD Geode LX driver)
>>> in http://cgit.freedesktop.org/xorg/driver/xf86-video-geode/tree/src/lx_display.c#n386
>>> then the rotate_mem offset and shadow_allocate address will be return to xf86CrtcSetModeTransform, after run to xf86RotatePrepare, it will call lx_crtc_shadow_create -> GetScratchPixmapHeader -> exaModifyPixmapHeader_classic to write the pPixData. The pPixData will be allocated to the sys_ptr and fb_ptr(if the judge come into existence).
>>>
>>
>> I am looking at your driver, and it's all a bit confusing. This will
>> have to wait until (at least) this evening.
>>
>>> Thanks
>>> Hunk Cui
>>>
>>> -----Original Message-----
>>> From: Maarten Maathuis [mailto:madman2003 at gmail.com]
>>> Sent: Wednesday, June 09, 2010 6:16 PM
>>> To: Cui, Hunk
>>> Cc: xorg-devel at lists.x.org; Huang, FrankR; Writer, Tim; Torres, Rigo
>>> Subject: Re: Who can explain the diff between Xserver-1.6.4 version and >1.7 version about the ExaGetPixmapAddress?
>>>
>>> On Wed, Jun 9, 2010 at 11:59 AM, Cui, Hunk <Hunk.Cui at amd.com> wrote:
>>>> Hi, Maarten,
>>>>        You can see my attachment screenshot, in this example, when run to the line 177-178, the pPixData is 0xb62b000, pExaScr->info->memoryBase is 0xb5e2b000,pExaScr->info->memorySize is 17825792, if only have "<", the judge will not come into existence.
>>>>        You can try it. If not come into existence. The fb_ptr address value is 0x0.
>>>
>>> How are you allocating rotatedData?
>>>
>>>>
>>>> Thanks,
>>>> Hunk Cui
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Maarten Maathuis [mailto:madman2003 at gmail.com]
>>>> Sent: Wednesday, June 09, 2010 5:47 PM
>>>> To: Cui, Hunk
>>>> Cc: xorg-devel at lists.x.org; Huang, FrankR; Writer, Tim; Torres, Rigo
>>>> Subject: Re: Who can explain the diff between Xserver-1.6.4 version and >1.7 version about the ExaGetPixmapAddress?
>>>>
>>>> On Wed, Jun 9, 2010 at 10:23 AM, Cui, Hunk <Hunk.Cui at amd.com> wrote:
>>>>> Hi, Maarten,
>>>>>
>>>>>         As your commit:
>>>>> http://cgit.freedesktop.org/xorg/xserver/commit/?id=12aeddf5ad41902a180f8108623f356642b3e911
>>>>>
>>>>>         About Scratch pixmap with gpu memory – Framebuffer. Now in > 1.7
>>>>> version, the exaModifyPixmapHeader function have been become
>>>>> exaModifyPixmapHeader_classic (
>>>>> http://cgit.freedesktop.org/xorg/xserver/tree/exa/exa_classic.c?id=ac7ac913fd98ea359c05c89968ab53a3223615b4
>>>>> Line 144).
>>>>>
>>>>>         When I debug to this point (my AMD xf86-video-geode), the line
>>>>> 171-172, why only have the “<” judge. For general, I think “((CARD8
>>>>> *)pPixData - pExaScr->info->memoryBase) <= pExaScr->info->memorySize)”,
>>>>> because rotate_mem offset should equal to the pExaScr->info->memorySize
>>>>> (e.g: displayed frame buffer). If only have the “<”, It will not go into
>>>>> this judge, so the fb_ptr address value is 0x0, then the value will affect
>>>>> the exaGetPixmapOffset function (in exa.c
>>>>> http://cgit.freedesktop.org/xorg/xserver/tree/exa/exa.c?id=ac7ac913fd98ea359c05c89968ab53a3223615b4
>>>>> line 62) about “return (CARD8 *)pExaPixmap->fb_ptr -
>>>>> pExaScr->info->memoryBase”, the range will be exceed, if run the
>>>>> driver_do_composite (It is lx_do_composite in our geode_driver), the
>>>>> dstOffset is a error value.
>>>>>
>>>>>         Could you explain this issue? Let me know why only have “<”.
>>>>
>>>> If your memory base is 20000, your offscreen memory size is 10000,
>>>> then 20000 to 29999 are valid addresses. If you enter 30000 - 10000 <=
>>>> 20000, then that would be true, which is wrong, that's why it's just
>>>> "<", because addressing starts from 0 not 1.
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Hunk Cui
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Life spent, a precious moment, in the wink of an eye we live and we die.
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Life spent, a precious moment, in the wink of an eye we live and we die.
>>>
>>>
>>
>>
>>
>> --
>> Life spent, a precious moment, in the wink of an eye we live and we die.
>>
>>
>
>
>
> --
> Life spent, a precious moment, in the wink of an eye we live and we die.
>
>

-- 
Life spent, a precious moment, in the wink of an eye we live and we die.