userptr support in drm drivers

[Daniel Vetter]

On Tue, Feb 16, 2016 at 09:33:51PM +0100, Christian König wrote:
> 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.

Yeah I like that flow. Jean, if you want to bring i915 into alignment with
radone by adding a I915_USERPTR option that selects MMU_NOTIFIER (probably
default y since vulkan needs this), then I very much want will merge it.
Distro kernels pretty much all select MMU_NOTIFIER already for unrelated
reasons, but it's good to be less suprising for everyone who builds their
own custom kernel.

Thanks, Daniel

> 
> 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,
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch