userptr support in drm drivers

[Christian König]

At least for Radeon and Amdgpu the current situation is actually what we 
want.

> However I still find it confusing
> that full userptr support is under #if defined(CONFIG_MMU_NOTIFIER),
> and not under #if defined(CONFIG_RADEON_USERPTR), resp. #if
> defined(CONFIG_AMDGPU_USERPTR). It means that full userptr support may
> be included even if these options are disabled.
Which is perfectly fine. Userptr support should be enabled when 
MMU_NOTIFIER is available.

How this becomes available is a different story. You can explicitly 
enable it which then pulls in the MMU_NOTIFIER dependency or you just 
enable it when you have the notfier anyway because of some other dependency.

That we have two options doing the same is just a matter of branching of 
amdgpu from radeon and not cleaning up the configuration options. So 
feel free to cleaning this up and write a patch which makes pulling in 
MMU_NOTIFIER as a general DRM option.

Regards,
Christian.

Am 16.02.2016 um 20:58 schrieb Jean Delvare:
> Hi all,
>
> While checking the openSUSE kernel configuration, I noticed a couple
> oddities regarding userptr support in the i915, radeon and amdgpu drm
> drivers. I'll like to discuss the current situation and come up with an
> agreement on how this could be cleaned up.
>
> Firstly, i915. This driver has userptr code under #if
> defined(CONFIG_MMU_NOTIFIER), however it neither selects this option nor
> depends on it. Given that CONFIG_MMU_NOTIFIER is not a user-visible
> option (it can only be selected by other kernel configuration options),
> it means that you get full userptr support or not depending on other
> unrelated kernel options. This isn't good.
>
> Secondly, radeon and amdgpu. They are slightly better in that they have
> Kconfig options selecting CONFIG_MMU_NOTIFIER (DRM_RADEON_USERPTR and
> DRM_AMDGPU_USERPTR respectively.) However I still find it confusing
> that full userptr support is under #if defined(CONFIG_MMU_NOTIFIER),
> and not under #if defined(CONFIG_RADEON_USERPTR), resp. #if
> defined(CONFIG_AMDGPU_USERPTR). It means that full userptr support may
> be included even if these options are disabled.
>
> I am not too familiar with MMU and this userptr stuff, but from where I
> stand, only two approaches make sense:
>
> * Either there is a good reason why people may want to disable full
>    userptr support. In this case options CONFIG_RADEON_USERPTR and
>    CONFIG_AMDGPU_USERPTR should really enable the code in question, it
>    should not be built without these options. And a similar option
>    should be introduced for the i915 driver to the same effect. Or
>    alternatively a single option for the whole DRM subsystem may make
>    more sense, as I doubt anyone would want to enable support in one
>    driver and disable it in another.
>
> * Or there is no specific reason why people would want to disable full
>    userptr support, in which case options CONFIG_RADEON_USERPTR and
>    CONFIG_AMDGPU_USERPTR should be removed and all 3 drivers should
>    unconditionally select CONFIG_MMU_NOTIFIER.
>
> If the sole purpose of these these settings is for development or
> debugging purposes, I'd go for option 1 with a run-time option to
> disable full userptr (drm.userptr=ro or some such.)
>
> As a general rule, fewer configuration options is better.
>
> Once a decision is made, I volunteer to write the patches.
>
> Thanks,