[PATCH 2/8] drm/radeon: don't use ttm bo->offset

[Nirmoy]

On 2/21/20 2:19 AM, Luben Tuikov wrote:
> On 2020-02-19 08:53, Nirmoy Das wrote:
>> Calculate GPU offset in radeon_bo_gpu_offset without depending on
>> bo->offset
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das at amd.com>
>> Reviewed-and-tested-by: Christian König <christian.koenig at amd.com>
>> ---
>>   drivers/gpu/drm/radeon/radeon.h        |  1 +
>>   drivers/gpu/drm/radeon/radeon_object.h | 16 +++++++++++++++-
>>   drivers/gpu/drm/radeon/radeon_ttm.c    |  4 +---
>>   3 files changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
>> index 30e32adc1fc6..b7c3fb2bfb54 100644
>> --- a/drivers/gpu/drm/radeon/radeon.h
>> +++ b/drivers/gpu/drm/radeon/radeon.h
>> @@ -2828,6 +2828,7 @@ extern void radeon_ttm_set_active_vram_size(struct radeon_device *rdev, u64 size
>>   extern void radeon_program_register_sequence(struct radeon_device *rdev,
>>   					     const u32 *registers,
>>   					     const u32 array_size);
>> +struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev);
>>   
>>   /*
>>    * vm
>> diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
>> index d23f2ed4126e..4d37571c7ff5 100644
>> --- a/drivers/gpu/drm/radeon/radeon_object.h
>> +++ b/drivers/gpu/drm/radeon/radeon_object.h
>> @@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo *bo)
>>    */
>>   static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo)
>>   {
>> -	return bo->tbo.offset;
>> +	struct radeon_device *rdev;
>> +	u64 start = 0;
>> +
>> +	rdev = radeon_get_rdev(bo->tbo.bdev);
>> +
>> +	switch(bo->tbo.mem.mem_type) {
> LKCS wants a space after a keyword, "switch (" .
Thanks Luben, will fix that for both radeon and amdgpu.
>
>> +	case TTM_PL_TT:
>> +		start = rdev->mc.gtt_start;
>> +		break;
>> +	case TTM_PL_VRAM:
>> +		start = rdev->mc.vram_start;
>> +		break;
>> +	}
> Could this lookup have been parameterized by "mem_type"
> to be looked up by an index (possibly "mem_type") to result
> in something new like (pseudo-code):
>       start = rdev->mc.mem_start_table[bo->tbo.mem.mem_type];
> Where "mem_start_table" is a new table holding memory starts
> of particular memories.
> Then you don't need the switch-case.

That would look good but I am afraid, amdgpu_ttm_domain_start() and 
radeon_bo_gpu_offset() will be the only consumer of /mem_start_table./

I think it is not worth to do a intrusive change for this patch series.

Regards,

Nirmoy


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20200221/c5eaafdd/attachment.htm>