server 1.7 EXA and R600 UploadToScreen infinite loop

[Dave Airlie]

On Wed, 2009-09-30 at 09:34 +0200, Michel Dänzer wrote:
> On Wed, 2009-09-30 at 13:08 +1000, Dave Airlie wrote: 
> > 
> > So I've been getting a recusive problem with 1.7's EXA (not with master)
> > 0369eeeb6bf8a808fa2df503fc8b8df81e6e07b8 works around the issue.
> > 
> > What seems to be happening is the r600 UTS has an "optimisation" (Alex did
> > you write it?)
> > 
> >     /* If we know the BO won't be busy, don't bother */
> >     if (driver_priv->bo->cref == 1 &&
> >     	!radeon_bo_is_busy(driver_priv->bo, &dst_domain))
> >     	return FALSE;
> > 
> > Now we get into a cycle, as shown below, the EXA core tries to migrate
> > to the FB, UTS fails it fallbacks to doing prepare/memcpy/finish, finish
> > notices dirty status, and tries to upload to screen again, which fallsback
> > to doing prepare/memcpy/finish, etc.
> > 
> > The patch in master works around this of course, however I've no idea whether
> > a) that is in fact the correct solution
> 
> I'm not sure it can prevent the problem in all cases.
> 
> Does the patch below help? The idea is to move the FinishAccess call
> after the valid region updates in exaCopyDirty(), so the second call of
> that should be a no-op. It would be better to stop the recursion
> earlier, but I don't have any good ideas offhand.
> 
> 
> diff --git a/exa/exa_migration_classic.c b/exa/exa_migration_classic.c
> index 4819af8..6d7b9f5 100644
> --- a/exa/exa_migration_classic.c
> +++ b/exa/exa_migration_classic.c
> @@ -242,11 +242,6 @@ exaCopyDirty(ExaMigrationPtr migrate, RegionPtr pValidDst, RegionPtr pValidSrc,
>  	pBox++;
>      }
>  
> -    if (access_prepared)
> -	exaFinishAccess(&pPixmap->drawable, fallback_index);
> -    else if (need_sync && sync)
> -	sync (pPixmap->drawable.pScreen);
> -
>      pExaPixmap->offscreen = save_offscreen;
>      pPixmap->devKind = save_pitch;
>  
> @@ -261,6 +256,11 @@ exaCopyDirty(ExaMigrationPtr migrate, RegionPtr pValidDst, RegionPtr pValidSrc,
>      REGION_UNION(pScreen, pValidDst, pValidDst, &CopyReg);
>  
>      REGION_UNINIT(pScreen, &CopyReg);
> +
> +    if (access_prepared)
> +	exaFinishAccess(&pPixmap->drawable, fallback_index);
> +    else if (need_sync && sync)
> +	sync (pPixmap->drawable.pScreen);
>  }
>  
>  /**
> 
> 

Yup this works fine, I'll resend it to the list from you and get Peter
to take it.

Dave.