On Tue, 5 Apr 2022 at 11:25, Christian König christian.koenig@amd.com wrote:
Am 29.03.22 um 18:02 schrieb Daniel Vetter:
On Mon, Mar 21, 2022 at 02:58:56PM +0100, Christian König wrote: [SNIP]
static unsigned long ttm_bo_io_mem_pfn(struct ttm_buffer_object *bo, diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c index f999fdd927df..c6d02c98a19a 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c @@ -1163,12 +1163,6 @@ int vmw_resources_clean(struct vmw_buffer_object *vbo, pgoff_t start, *num_prefault = __KERNEL_DIV_ROUND_UP(last_cleaned - res_start, PAGE_SIZE); vmw_bo_fence_single(bo, NULL);
if (bo->moving)dma_fence_put(bo->moving);return dma_resv_get_singleton(bo->base.resv,DMA_RESV_USAGE_KERNEL,&bo->moving);This seems to be entirely misplaced and I'm pretty sure doesn't even compile interim.
Mhm, removing that is correctly placed as far as I can see.
What VMWGFX does here is to update bo->moving to please TTM, but since we now drop the bo->moving fence from TTM and always wait for all fences with DMA_RESV_USAGE_KERNEL before allowing CPU access that workaround isn't necessary any more.
Hm yeah that makes sense. Just out of paranoid would be good if you can get an ack on the previous patch that downgrades from USAGE_WRITE to USAGE_KERNEL here from vmwgfx folks, but I guess that should be fine.
Just from reading the commit that introduce this is looks a little bit like the intent is actually to make any USAGE_WRITE a mandatory fence you can never cheat out of, but maybe I got this all wrong.
} return 0;diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index c17b2df9178b..4c7134550262 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -97,7 +97,6 @@ struct ttm_tt;
- @lru: List head for the lru list.
- @ddestroy: List head for the delayed destroy list.
- @swap: List head for swap LRU list.
- @moving: Fence set when BO is moving
- @offset: The current GPU offset, which can have different meanings
- depending on the memory type. For SYSTEM type memory, it should be 0.
- @cur_placement: Hint of current placement.
@@ -150,7 +149,6 @@ struct ttm_buffer_object { * Members protected by a bo reservation. */
- struct dma_fence *moving; unsigned priority; unsigned pin_count;
Aside from the vmwgfx thing this looks good. With the vmwgfx patch split issue (I think it's just that) fixed:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Is it enough if I explain why we update VMWGFX in the commit message?
Yeah sounds fine to me, this patch isn't a functional change now that you explained it that's clear. I'm still wondering whether there's something fishy going on, but that's on earlier patches. -Daniel
Thanks, Christian.
-- 2.25.1