On Wed, Jan 31, 2018 at 6:27 PM, Felix Kuehling <[email protected]> wrote: > On 2018-01-31 10:29 AM, Oded Gabbay wrote: >> On Wed, Jan 31, 2018 at 5:23 PM, Deucher, Alexander >> <[email protected]> wrote: >>> ________________________________ >>> From: amd-gfx <[email protected]> on behalf of Oded >>> Gabbay <[email protected]> >>> 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 <[email protected]> >>> wrote: >>>> Recognize dGPU ASIC families. >>>> >>>> Signed-off-by: Felix Kuehling <[email protected]> >>>> --- >>>> 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.
I agree its a lot of churn but I don't think its a total waste of time, even if those devices are not really important anymore. I'll try to find some time to do it. Oded > > 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 <[email protected]> >>> _______________________________________________ >>> amd-gfx mailing list >>> [email protected] >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
