userptr support in drm drivers

[Daniel Vetter]

On Fri, Feb 19, 2016 at 03:58:24PM +0100, Christian König wrote:
> >If a minority of users need to opt out from MMU_NOTIFIER, then
> >DRM_RADEON_USERPTR and friends should default to y and be hidden behind
> >EXPERT. That way you avoid asking yet more questions to everyone else.
> >Would that work for you?
> No, just the other way around.
> 
> When MMU_NOTIFIER is selected anyway we actually don't want to display the
> option, but that's not possible for none menu entries IIRC the Kconfig
> correctly.
> 
> If somebody configures the kernel and don't selects any of the other
> dependencies for MMU_NOTIFIER the feature should not be available by
> default.
> 
> Making it an expert option is fine with me.

Concurred, if we do this across all drivers supporting userptr.
-Daniel

> 
> Regards,
> Christian.
> 
> Am 19.02.2016 um 15:11 schrieb Jean Delvare:
> >Hi Christian,
> >
> >On Thu, 18 Feb 2016 10:48:18 +0100, Christian König wrote:
> >>Am 18.02.2016 um 09:59 schrieb Jean Delvare:
> >>>Maybe I was not clear enough in my original post, but I am really
> >>>advocating for FEWER kconfig options, not more. Plus I just explained
> >>>why I think the radeon and amdgpu drivers do it wrong, so I'm certainly
> >>>not going to align i915 on that.
> >>>
> >>>>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.
> >>>The current situation is actually very surprising and this is what I
> >>>would like to change. The options of one driver depending on choices
> >>>made somewhere else is utterly confusing. CONFIG_RADEON_USERPTR and
> >>>CONFIG_AMDGPU_USERPTR give the user a little more control where i915
> >>>gives none, but that's only in one direction (you can force full
> >>>userptr support in, but you can't force it out.)
> >>As I said, that is perfectly fine and exactly what we want.
> >I don't know what "we" represent in your sentence, but I doubt a
> >majority of users want that.
> >
> >>>I'm still waiting for someone to explain why we can't just
> >>>unconditionally select MMU_NOTIFIER in these 3 drivers and be done with
> >>>it. That's less work at kernel configuration time AND fewer
> >>>preprocessing conditionals within the code, so easier/better testing
> >>>coverage and more readable code. We would need a very good reason for
> >>>NOT doing it.
> >>Adding the MMU Notifier causes overhead in the MM which people want to
> >>avoid when they don't need userptr support.
> >>
> >>We used to select MMU_NOTIFIER unconditionally before and explicitly
> >>added this option on user request. So yes it is clearly necessary.
> >Thanks for the explanation.
> >
> >Can you tell me more about the overhead? Is it actually measurable?
> >Which people need this degree of optimization?
> >
> >Are the i915, radeon or amdgpu drm drivers used on embedded systems?
> >
> >Just because people asked for something doesn't mean that implementing
> >it is a good idea. Keeping things simple is a valid goal on its own.
> >Anyone could ask for features they don't need to become options they
> >can disable, but in the long run this approach can't be sustained.
> >
> >>The additional userptr support code in the drivers are negligible so
> >>always enabling the code when the MMU Notifier is available is also
> >>perfectly fine.
> >Indeed this is a small amount of code. OTOH you have ifdefs in place
> >anyway. Using the right ones would turn the option into a real option,
> >i.e. the user decides to enable a feature or not. Not a "you may or may
> >not get the feature" option as we have at the moment. More control to
> >the user and less confusion.
> >
> >>So if you want to clean it up make a common DRM option which selects MMU
> >>Notifier if it isn't already selected.
> >I will, as it will still be an improvement compared to the current
> >situation. But I still believe this is confusing in its current form.
> >
> >If a minority of users need to opt out from MMU_NOTIFIER, then
> >DRM_RADEON_USERPTR and friends should default to y and be hidden behind
> >EXPERT. That way you avoid asking yet more questions to everyone else.
> >Would that work for you?
> >
> 

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