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

[Kuehling, Felix]

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


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
>