drm/i915: Add an optional selection from i915 of CONFIG_MMU_NOTIFIER
diff mbox

Message ID 1437481008-2064-1-git-send-email-chris@chris-wilson.co.uk
State New
Headers show

Commit Message

Chris Wilson July 21, 2015, 12:16 p.m. UTC
userptr requires mmu-notifier for full unprivileged support. Most
systems have mmu-notifier support already enabled as a requirement for
virtualisation support, but we should make the option for i915 to take
advantage of mmu-notifiers explicit (and enable by default so that
regular userspace can take advantage of passing client memory to the
GPU.)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Kconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Tvrtko Ursulin July 21, 2015, 12:31 p.m. UTC | #1
On 07/21/2015 01:16 PM, Chris Wilson wrote:
> userptr requires mmu-notifier for full unprivileged support. Most
> systems have mmu-notifier support already enabled as a requirement for
> virtualisation support, but we should make the option for i915 to take
> advantage of mmu-notifiers explicit (and enable by default so that
> regular userspace can take advantage of passing client memory to the
> GPU.)
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/Kconfig | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index eb87e2538861..1e33195e4a12 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -62,3 +62,14 @@ config DRM_I915_PRELIMINARY_HW_SUPPORT
>   	  option changes the default for that module option.
>
>   	  If in doubt, say "N".
> +
> +config DRM_I915_USERPTR
> +	bool "Always enable userptr support"
> +	depends on DRM_I915
> +	select MMU_NOTIFIER
> +	default y
> +	help
> +	  This option selects CONFIG_MMU_NOTIFIER if it isn't already
> +	  selected to enabled full userptr support.

s/enabled/enable/

I think it needs more text under help to explain what are you giving up 
if N, or to justify Y in other words. For distro kernels it is 
irrelevant but advanced users will want to know.

But since overall I think it is justified, in principle:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
Daniel Vetter July 21, 2015, 2:17 p.m. UTC | #2
On Tue, Jul 21, 2015 at 01:31:47PM +0100, Tvrtko Ursulin wrote:
> 
> On 07/21/2015 01:16 PM, Chris Wilson wrote:
> >userptr requires mmu-notifier for full unprivileged support. Most
> >systems have mmu-notifier support already enabled as a requirement for
> >virtualisation support, but we should make the option for i915 to take
> >advantage of mmu-notifiers explicit (and enable by default so that
> >regular userspace can take advantage of passing client memory to the
> >GPU.)
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >---
> >  drivers/gpu/drm/i915/Kconfig | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> >index eb87e2538861..1e33195e4a12 100644
> >--- a/drivers/gpu/drm/i915/Kconfig
> >+++ b/drivers/gpu/drm/i915/Kconfig
> >@@ -62,3 +62,14 @@ config DRM_I915_PRELIMINARY_HW_SUPPORT
> >  	  option changes the default for that module option.
> >
> >  	  If in doubt, say "N".
> >+
> >+config DRM_I915_USERPTR
> >+	bool "Always enable userptr support"
> >+	depends on DRM_I915
> >+	select MMU_NOTIFIER
> >+	default y
> >+	help
> >+	  This option selects CONFIG_MMU_NOTIFIER if it isn't already
> >+	  selected to enabled full userptr support.
> 
> s/enabled/enable/
> 
> I think it needs more text under help to explain what are you giving up if
> N, or to justify Y in other words. For distro kernels it is irrelevant but
> advanced users will want to know.
> 
> But since overall I think it is justified, in principle:
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Why not just select MMU_NOTIFIER and be done? Yes there's more overhead
but apparently everyone wants them anyway, so I don't see much point. And
it would give us nicely uniform test coverage, yay!
-Daniel
Chris Wilson July 21, 2015, 2:28 p.m. UTC | #3
On Tue, Jul 21, 2015 at 04:17:05PM +0200, Daniel Vetter wrote:
> On Tue, Jul 21, 2015 at 01:31:47PM +0100, Tvrtko Ursulin wrote:
> > 
> > On 07/21/2015 01:16 PM, Chris Wilson wrote:
> > >userptr requires mmu-notifier for full unprivileged support. Most
> > >systems have mmu-notifier support already enabled as a requirement for
> > >virtualisation support, but we should make the option for i915 to take
> > >advantage of mmu-notifiers explicit (and enable by default so that
> > >regular userspace can take advantage of passing client memory to the
> > >GPU.)
> > >
> > >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > >---
> > >  drivers/gpu/drm/i915/Kconfig | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > >diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> > >index eb87e2538861..1e33195e4a12 100644
> > >--- a/drivers/gpu/drm/i915/Kconfig
> > >+++ b/drivers/gpu/drm/i915/Kconfig
> > >@@ -62,3 +62,14 @@ config DRM_I915_PRELIMINARY_HW_SUPPORT
> > >  	  option changes the default for that module option.
> > >
> > >  	  If in doubt, say "N".
> > >+
> > >+config DRM_I915_USERPTR
> > >+	bool "Always enable userptr support"
> > >+	depends on DRM_I915
> > >+	select MMU_NOTIFIER
> > >+	default y
> > >+	help
> > >+	  This option selects CONFIG_MMU_NOTIFIER if it isn't already
> > >+	  selected to enabled full userptr support.
> > 
> > s/enabled/enable/
> > 
> > I think it needs more text under help to explain what are you giving up if
> > N, or to justify Y in other words. For distro kernels it is irrelevant but
> > advanced users will want to know.
> > 
> > But since overall I think it is justified, in principle:
> > 
> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Why not just select MMU_NOTIFIER and be done? Yes there's more overhead
> but apparently everyone wants them anyway, so I don't see much point. And
> it would give us nicely uniform test coverage, yay!

In a few years, when mmu-notifiers are enabled by default.
-Chris

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index eb87e2538861..1e33195e4a12 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -62,3 +62,14 @@  config DRM_I915_PRELIMINARY_HW_SUPPORT
 	  option changes the default for that module option.
 
 	  If in doubt, say "N".
+
+config DRM_I915_USERPTR
+	bool "Always enable userptr support"
+	depends on DRM_I915
+	select MMU_NOTIFIER
+	default y
+	help
+	  This option selects CONFIG_MMU_NOTIFIER if it isn't already
+	  selected to enabled full userptr support.
+
+	  If in doubt, say "Y".