[PATCH] exa: fix gnome-panel corruption

[Maarten Maathuis]

2010/2/11 Michel Dänzer <michel at daenzer.net>:
>
> Thanks for tackling this, Maarten!
>
> On Thu, 2010-02-11 at 19:05 +0100, Maarten Maathuis wrote:
>> - A mapped pixmap can't be used for acceleration, any decent memory manager
>> will refuse this.
>> - Source pixmaps may need updating, so move in the pixmap unconditionally, it
>> should be a no-op in most cases anyway.
>
> 'May need'? Have you actually observed this in practice? Only the mixed
> pixmap pointed to by pExaScr->deferred_mixed_pixmap should ever have
> valid bits in the system memory copy with no corresponding valid bits in
> the GPU copy. In which case the new code could still be conditionalized
> on

Yes i have observed this in practice, the reason is "fairly" obvious.

Do software rendering on full pixmap, do hardware rendering as src
with preg (driver pixmap created), do software rendering (sw pixmap is
killed, but not everything is in hardware pixmap yet).

>
> if (pExaScr->deferred_mixed_pixmap == pPixmap)
>
> (the part before the && was never necessary as pPixmap can never be NULL
> here).
>
> It should be possible to restructure the code such that the late_failure
> label and goto aren't necessary.

You cannot assume the prepare access to succeed the 2nd time, although
it will in 99.99% of the time.

>
> Is there a bug report that can be referenced in the commit message?

Not to my knowledge.

>
>
>> Signed-off-by: Maarten Maathuis <madman2003 at gmail.com>
>> ---
>>  exa/exa_migration_mixed.c |   18 ++++++++++++------
>>  1 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/exa/exa_migration_mixed.c b/exa/exa_migration_mixed.c
>> index 6816e6c..77b2b2b 100644
>> --- a/exa/exa_migration_mixed.c
>> +++ b/exa/exa_migration_mixed.c
>> @@ -167,9 +167,12 @@ exaPrepareAccessReg_mixed(PixmapPtr pPixmap, int index, RegionPtr pReg)
>>      ExaPixmapPriv(pPixmap);
>>
>>      if (!ExaDoPrepareAccess(pPixmap, index)) {
>> -     Bool has_gpu_copy = exaPixmapHasGpuCopy(pPixmap);
>> +     Bool has_gpu_copy;
>>       ExaMigrationRec pixmaps[1];
>>
>> +late_failure:
>> +     has_gpu_copy = exaPixmapHasGpuCopy(pPixmap);
>> +
>>       /* Do we need to allocate our system buffer? */
>>       if (!pExaPixmap->sys_ptr) {
>>           pExaPixmap->sys_ptr = malloc(pExaPixmap->sys_pitch *
>> @@ -231,12 +234,15 @@ exaPrepareAccessReg_mixed(PixmapPtr pPixmap, int index, RegionPtr pReg)
>>      /* We have a gpu pixmap that can be accessed, we don't need the cpu copy
>>       * anymore. Drivers that prefer DFS, should fail prepare access. */
>>      } else if (pExaPixmap->pDamage && exaPixmapHasGpuCopy(pPixmap)) {
>> -     ExaScreenPriv(pPixmap->drawable.pScreen);
>>
>> -     /* Copy back any deferred content if needed. */
>> -     if (pExaScr->deferred_mixed_pixmap &&
>> -         pExaScr->deferred_mixed_pixmap == pPixmap)
>> -         exaMoveInPixmap_mixed(pPixmap);
>> +     /* You cannot do accelerated operations while a buffer is mapped. */
>> +     exaFinishAccess(&pPixmap->drawable, index);
>> +     /* Sources with pReg are not fully in the gpu pixmap yet, as well
>> +      * as deferred destination pixmaps.
>> +      */
>> +     exaMoveInPixmap_mixed(pPixmap);
>> +     if (!ExaDoPrepareAccess(pPixmap, index))
>> +         goto late_failure;
>>
>>       DamageUnregister(&pPixmap->drawable, pExaPixmap->pDamage);
>>       DamageDestroy(pExaPixmap->pDamage);
>
>
> --
> Earthling Michel Dänzer           |                http://www.vmware.com
> Libre software enthusiast         |          Debian, X and DRI developer
>