[PATCH 03/19] drm/amdkfd: Remove bogus divide-by-sizeof(uint32_t)

[Oded Gabbay]

On Sat, Aug 12, 2017 at 9:02 PM, Kuehling, Felix <Felix.Kuehling at amd.com> wrote:
> Hi Oded,
>
> Most of our recent work has been on the amdgpu driver. We basically just keep radeon compiling these days. Amdgpu can support all the GPUs that KFD supports. Between the amdgpu developers and the KFD team we've also talked about merging KFD into amdgpu at some point and replacing the KFD2KGD function pointer interfaces with mostly direct function calls. This would remove KFD support from radeon.
>
> What's your position on radeon KFD support going forward? Do you insist in maintaining it, just for Kaveri?
>
> Regards,
>   Felix
>
If amdgpu supports Kaveri, then there is no need to maintain radeon
support forever. As long as we provide a migration option, then we can
say that kfd won't support radeon starting from kernel version x.y




>
> From: Oded Gabbay <oded.gabbay at gmail.com>
> Sent: Saturday, August 12, 2017 8:37 AM
> To: Kuehling, Felix
> Cc: amd-gfx list
> Subject: Re: [PATCH 03/19] drm/amdkfd: Remove bogus divide-by-sizeof(uint32_t)
>
> On Sat, Aug 12, 2017 at 12:56 AM, Felix Kuehling <Felix.Kuehling at amd.com> wrote:
>> kfd2kgd->address_watch_get_offset returns dword register offsets.
>> The divide-by-sizeof(uint32_t) is incorrect.
>
> In amdgpu that's true, but in radeon that's incorrect.
> If you look at cik_reg.h in radeon driver, you will see the address of
> all TCP_WATCH_* registers is multiplied by 4, and that's why Yair
> originally divided the offset by sizeof(uint32_t).
> I think this patch should move the divide-by-sizeof operation into the
> radeon function instead of just deleting it from kfd_dbgdev.c.
>
> Oded
>
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 8 --------
>>  1 file changed, 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>> index 8b14a4e..faa0790 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>> @@ -442,8 +442,6 @@ static int dbgdev_address_watch_diq(struct kfd_dbgdev *dbgdev,
>>                                         i,
>>                                         ADDRESS_WATCH_REG_CNTL);
>>
>> -               aw_reg_add_dword /= sizeof(uint32_t);
>> -
>>                 packets_vec[0].bitfields2.reg_offset =
>>                                         aw_reg_add_dword - AMD_CONFIG_REG_BASE;
>>
>> @@ -455,8 +453,6 @@ static int dbgdev_address_watch_diq(struct kfd_dbgdev *dbgdev,
>>                                         i,
>>                                         ADDRESS_WATCH_REG_ADDR_HI);
>>
>> -               aw_reg_add_dword /= sizeof(uint32_t);
>> -
>>                 packets_vec[1].bitfields2.reg_offset =
>>                                         aw_reg_add_dword - AMD_CONFIG_REG_BASE;
>>                 packets_vec[1].reg_data[0] = addrHi.u32All;
>> @@ -467,8 +463,6 @@ static int dbgdev_address_watch_diq(struct kfd_dbgdev *dbgdev,
>>                                         i,
>>                                         ADDRESS_WATCH_REG_ADDR_LO);
>>
>> -               aw_reg_add_dword /= sizeof(uint32_t);
>> -
>>                 packets_vec[2].bitfields2.reg_offset =
>>                                 aw_reg_add_dword - AMD_CONFIG_REG_BASE;
>>                 packets_vec[2].reg_data[0] = addrLo.u32All;
>> @@ -485,8 +479,6 @@ static int dbgdev_address_watch_diq(struct kfd_dbgdev *dbgdev,
>>                                         i,
>>                                         ADDRESS_WATCH_REG_CNTL);
>>
>> -               aw_reg_add_dword /= sizeof(uint32_t);
>> -
>>                 packets_vec[3].bitfields2.reg_offset =
>>                                         aw_reg_add_dword - AMD_CONFIG_REG_BASE;
>>                 packets_vec[3].reg_data[0] = cntl.u32All;
>> --
>> 2.7.4
>>
>