From: Jerome G. <gl...@fr...> - 2009-07-30 08:10:44
|
On Wed, 2009-07-29 at 21:53 +0200, Thomas Hellström wrote: > Jerome Glisse skrev: > > Hi Thomas, > > > > I think ttm is doing somethings wrong when we validate a bo for the > > same mem type but with different cache flags. Here is my understanding : > > > > (Use case i am refering too is object validate with > > current state GTT | CACHED validate to GTT | WC) > > > > if (!ttm_bo_mem_compat(bo->proposed_placement, &bo->mem) > > Is evaluate as true because cache flags are differents, For instance > > is it goes from UNCACHED to WC or CACHED. > > > > So ttm_bo_move_buffer get call, which call ttm_bo_handle_move_memin there caching change will only happen if there is no ttm allocated > > which might not be the case if for instance we are dealing with > > GTT memory (so i think cache transitioning of the object is wrong > > and we should move ttm_tt_set_placement_caching outside the if > > in ttm_bo_handle_move_mem). > > > In this particular case, ttm_bo_handle_move_mem will first call > ttm_bo_unmap_virtual(), then go ahead and call ttm_bo_move_ttm(). This > routine will unbind from GTT, change caching and the rebind to GTT. > > > ttm_bo_handle_move_mem will end up calling driver callback move > > routine > No, it will use ttm_bo_move_ttm(). > > > which will call in radeon case the ttm_bo_move_accel_cleanup > > and which will create a ghost object holding the ttm memory in order > > to delete it at latter point (which is wrong as this memory is still > > needed). > > > > So shouldn't the ttm_bo_move_buffer catch this and not call device > > move callback but simply transition cache setup of the object ? > > Or do i misunderstand somethings ? > > > > > It should call ttm_bo_move_ttm() and hopefully it does. However, it is a > bit wasteful of GTT space since I think it actually allocates a new GTT > slot before freeing the old one. Ideally I think it should reuse the old > slot. There's some optimization to be done there. > > So why does it bother unbinding from GTT in the first place? This is > because translation tables that support both snooped and unsnooped > bindings usually need to rewrite the page tables to reflect this change. > > > Cheers, > > Jerome > > > > > /Thomas I need to do more debugging but i am seeing again massive pte corruption and i thought i spotted the right path. Jerome |