Re: [PATCH 23/23] drm/ttm: remove bo->moving

["Christian König" <christian.koenig@amd.com>]

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.

>>   	}
>>   
>>   	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?

Thanks,
Christian.

>
>>   
>> -- 
>> 2.25.1
>>