Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset

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

Am 13.02.20 um 15:30 schrieb Gerd Hoffmann:
>> @@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo,
>>   		(bo->tbo.mem.mem_type == TTM_PL_VRAM)
>>   		? &qdev->main_slot : &qdev->surfaces_slot;
>>   
>> -	WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset);
>> -
>> -	/* TODO - need to hold one of the locks to read tbo.offset */
>> -	return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset);
>> +	return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) +
>> +				  slot->gpu_offset + offset);
>>   }
> --verbose please.

The warning and the TODO should probably stay, don't they?

> I don't get the logic behind this change.

We ran into the problem that the whole offset handling in TTM is 
actually completely hardware specific.

E.g. Some need pfn, some need byte offsets, some need 32bit, some 64bit 
and some don't have a consistent offset at all (e.g. amdgpu for exmaple).

So we try to move that back into the drivers to concentrate on memory 
management in TTM.

>
> The other chunks look sane, calculating slot->gpu_offset
> in setup_slot() certainly makes sense.

Thanks for the review,
Christian.

>
> cheers,
>    Gerd
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel