[PATCH 9/9] drm/amdgpu: Enable KFD initialization on dGPUs

[Oded Gabbay]

On Wed, Jan 31, 2018 at 5:34 PM, Christian König
<christian.koenig at amd.com> wrote:
> Am 31.01.2018 um 16:31 schrieb Oded Gabbay:
>>
>> On Wed, Jan 31, 2018 at 5:28 PM, Christian König
>> <ckoenig.leichtzumerken at gmail.com> wrote:
>>>
>>> Am 31.01.2018 um 16:25 schrieb Oded Gabbay:
>>>>
>>>> On Fri, Jan 5, 2018 at 12:17 AM, Felix Kuehling <Felix.Kuehling at amd.com>
>>>> wrote:
>>>>>
>>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>>>>> ---
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 +++++
>>>>>    1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>>>> index 335e454..7ebe430 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>>>> @@ -78,10 +78,15 @@ void amdgpu_amdkfd_device_probe(struct
>>>>> amdgpu_device
>>>>> *adev)
>>>>>           switch (adev->asic_type) {
>>>>>    #ifdef CONFIG_DRM_AMDGPU_CIK
>>>>>           case CHIP_KAVERI:
>>>>> +       case CHIP_HAWAII:
>>>>>                   kfd2kgd = amdgpu_amdkfd_gfx_7_get_functions();
>>>>>                   break;
>>>>>    #endif
>>>>>           case CHIP_CARRIZO:
>>>>> +       case CHIP_TONGA:
>>>>> +       case CHIP_FIJI:
>>>>> +       case CHIP_POLARIS10:
>>>>> +       case CHIP_POLARIS11:
>>>>
>>>> Polaris isn't gfx 9 ?
>>>> or is it called differently ?
>>>
>>>
>>> No Polaris are just updated gfx8 variants.
>>>
>>> gfx9 is Vega10.
>>>
>>> Christian.
>>
>> OK, thanks. So this patch is fine and is
>> Reviewed-by: Oded Gabbay <oded.gabbay at gmail.com>
>>
>> Having said that, as I wrote to Alex, if that is the case, I think we
>> should rename all soc-dependent functions from using cik and vi as
>> identifiers to gfx7/gfx8.
>
>
> Well that depends on what the name refers to.
>
> gfx7/gfx8 just describe the version of the CP, not the full ASIC.
>
> For example Tonga and Polaris are both gfx8, but have a different SDMA IIRC.
>
> Christian.

Understood, but see the following code from mqd_manager_init():

switch (dev->device_info->asic_family) {
case CHIP_KAVERI:
     return mqd_manager_init_cik(type, dev);
case CHIP_HAWAII:
     return mqd_manager_init_cik_hawaii(type, dev);
case CHIP_CARRIZO:
     return mqd_manager_init_vi(type, dev);
case CHIP_TONGA:
case CHIP_FIJI:
case CHIP_POLARIS10:
case CHIP_POLARIS11:
     return mqd_manager_init_vi_tonga(type, dev);
default:
WARN(1, "Unexpected ASIC family %u",
     dev->device_info->asic_family);
}


You started with mqd_manager_init_cik and mqd_manager_init_vi, which is nice.
Then, you add mqd_manager_init_cik_hawaii and
mqd_manager_init_vi_tonga, which is less nice.
Then, to top it off, you assign CHIP_POLARIS10 and CHIP_POLARIS11 to
mqd_manager_init_vi_tonga ? polaris is neither vi, nor tonga.

same thing in device_queue_manager_init()

That's what I meant by saying cik and vi aren't so good as identifiers.

Oded

>
>
>>
>> Oded
>>>>>
>>>>>                   kfd2kgd = amdgpu_amdkfd_gfx_8_0_get_functions();
>>>>>                   break;
>>>>>           default:
>>>>> --
>>>>> 2.7.4
>>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>>>
>