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 >>> >>> >