[PATCH] drm/amdgpu: select ARCH_HAS_HMM and ZONE_DEVICE option

[Yang, Philip]

Thanks Jerome for the the correct HMM config option, only select 
HMM_MIRROR is not good enough because CONFIG_HMM option maybe missing, 
add depends on ARCH_HAS_HMM will solve the issue.

I will submit new patch to fix the compilation error if HMM_MIRROR 
config is missing and the HMM config dependency issue.

Philip

On 2019-02-20 7:25 p.m., Jerome Glisse wrote:
> On Thu, Feb 21, 2019 at 12:17:33AM +0000, Kuehling, Felix wrote:
>> On 2019-02-20 6:34 p.m., Jerome Glisse wrote:
>>> On Wed, Feb 20, 2019 at 10:39:49PM +0000, Kuehling, Felix wrote:
>>>> On 2019-02-20 5:12 p.m., Jerome Glisse wrote:
>>>>> On Wed, Feb 20, 2019 at 07:18:17PM +0000, Kuehling, Felix wrote:
>>>>>> [+Jerome]
>>>>>>
>>>>>> Why to we need ZONE_DEVICE. I didn't think this was needed for mirroring
>>>>>> CPU page tables to device page tables.
>>>>>>
>>>>>> ARCH_HAS_HMM depends on (X86_64 || PPC64). Do we have some alternative
>>>>>> for ARM support?
>>>>>>
>>>>>> Also, the name ARCH_HAS_HMM looks like it's meant to be selected by the
>>>>>> CPU architecture rather than any driver. Jerome, do you have any advice?
>>>>> This patch is wrong you need to depend on ARCH_HAS_HMM and
>>>> Who selects ARCH_HAS_HMM? Currently I don't see this selected anywhere.
>>>> So any config option that depends on it will be invisible in menuconfig.
>>>> Do we need ARCH_HAS_HMM somewhere in the arch/x86/Kconfig and
>>>> arch/powerpc/Kconfig?
>>>>
>>>> Also, ARCH_HAS_HMM does not currently support ARM. Does that mean we
>>>> can't have ARM support in AMDGPU if we start using HMM?
>>> ARCH_HAS_HMM is defined by architecture that support HMM. So par x86
>>> and PPC. It should not be hard to add it to ARM (i can not remember if
>>> ARM has DAX yet or not, if ARM does not have DAX then you need to add
>>> that first).
>>
>> Not having ARM support is a bummer. I just enabled KFD on ARM a few
>> weeks ago. Now depending on HMM makes KFD unusable on ARM. [+Mark FYI] I
>> hope this is only a temporary setback.
> 
> It should not be hard to add in fact all it might need is a Kconfig
> patch. I have no easy access to ARM with PCIE so i have not tackle
> this yet.
> 
>>
>>
>>>> Finally, ARCH_HAS_HMM has a bunch of dependencies. If they are not met,
>>>> I guess it can't be enabled. Should those be "select"s instead?
>>> No they should not be selected, people configuring their system need
>>> to have the freedom of doing so. All those option are selected in all
>>> the big distribution.
>> As far as I can tell, the arch/x86/Kconfig doesn't select ARCH_HAS_HMM.
>> Its default is "y", so it should be enabled on anything that meets the
>> dependencies. But ZONE_DEVICE was not enabled by default. I think that's
>> what broke our kernel configs.
>>
>> We'll fix our own kernel configs to enable ZONE_DEVICE and ARCH_HAS_HMM
>> to get our internal builds to work again.
> 
> You seem to be doing weird thing with your kconfig ...
> 
>>
>> I suspect other users with their own kernel configs will stumble over
>> this and wonder why KFD and userptr support are disabled in their builds.
> 
> Patch to improve kconfig are welcome but they should not force select
> thing. Configuration is there to give user freedom to select fewature
> they want to give up.
> 
> Maybe following would help:
> ARCH_HAS_HMM
> -	bool
> -	default y
> +	def_bool y
> 
> Cheers,
> Jérôme
>