[PATCH 7/9] drm/amdkfd: Add dGPU support to kernel_queue_init

[Felix Kuehling]

On 2018-01-31 10:29 AM, Oded Gabbay wrote:
> On Wed, Jan 31, 2018 at 5:23 PM, Deucher, Alexander
> <Alexander.Deucher at amd.com> wrote:
>> ________________________________
>> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of Oded
>> Gabbay <oded.gabbay at gmail.com>
>> Sent: Wednesday, January 31, 2018 10:17 AM
>> To: Kuehling, Felix
>> Cc: amd-gfx list
>> Subject: Re: [PATCH 7/9] drm/amdkfd: Add dGPU support to kernel_queue_init
>>
>> On Fri, Jan 5, 2018 at 12:17 AM, Felix Kuehling <Felix.Kuehling at amd.com>
>> wrote:
>>> Recognize dGPU ASIC families.
>>>
>>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>>> index 5dc6567..69f4964 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>>> @@ -297,10 +297,15 @@ struct kernel_queue *kernel_queue_init(struct
>>> kfd_dev *dev,
>>>
>>>         switch (dev->device_info->asic_family) {
>>>         case CHIP_CARRIZO:
>>> +       case CHIP_TONGA:
>>> +       case CHIP_FIJI:
>>> +       case CHIP_POLARIS10:
>>> +       case CHIP_POLARIS11:
>> I believe POLARIS is from arcatic islands, no ?
>> Maybe rename kernel_queue_init_vi to kernel_queue_init_vi_ai ?
>> or create a new function kernel_queue_init_ai() and assign same
>> functions as vi ?
>> Either way, I think you need to address that.
>>
>> They are all gfx8.  adding ai just confuses things.

Internally we use VI and GFX8 interchangably. I think what's confusing
is, that internal code names are used for marketing purposes and applied
to the wrong chip generation.

Another precedent for that is Hawaii. It was called the first "volcanic
island" GPU when it was launched at an event on Hawaii (a volcanic
island), but as far as the driver is concerned, it belongs to the CIK
generation.

It's really hard to keep consistent naming, when naming conventions get
misappropriated to mean different things over time.

>>
>> Alex
> In that case, I think it is better maybe to change it to
> kernel_queue_init_gfx_7 and kernel_queue_init_gfx_8, to be consistent
> with the calls to amdgpu_amdkfd_gfx_7_0_get_functions and
> amdgpu_amdkfd_gfx_8_0_get_functions.
>
> Leaving as cik and vi as the identifier when it clearly isn't seems
> confusing to me as well.

For Vega10 we use the suffix _v9 instead of _cik or _vi. For consistency
and brevity I could rename _cik->v7 and _vi->v8. However, that would be
a lot of churn and, in my eyes, a waste of time.

Regards,
  Felix

>
> Oded
>
>>
>>>                 kernel_queue_init_vi(&kq->ops_asic_specific);
>>>                 break;
>>>
>>>         case CHIP_KAVERI:
>>> +       case CHIP_HAWAII:
>>>                 kernel_queue_init_cik(&kq->ops_asic_specific);
>>>                 break;
>>>         default:
>>> --
>>> 2.7.4
>>>
>> Other then that, This patch is:
>> Reviewed-by: Oded Gabbay <oded.gabbay at gmail.com>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx