[PATCH] drm/i915: refuse to load on gen6+ without kms

[PATCH] drm/i915: refuse to load on gen6+ without kms

[Daniel Vetter]

Spurred by an irc discussion, let's start to clear up which parts of
our kms + ums/gem + ums/dri1 + vbios/dri1 kernel driver pieces
userspace in the wild actually uses.

The idea is that we introduce checks at entry-points (module load
time, ioctls, ...) first and then reap any obviously dead code in a
second step.

As a first step refuse to load without kms on chips where userspace
never supported ums. Now upstream hasn't supported ums on ilk, ever.
But RHEL had the great idea to backport the kms support to their ums
driver.

Cc: Dave Airlie <airlied@gmail.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_dma.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 64dfbb8..8567fdf 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1955,9 +1955,17 @@ i915_mtrr_setup(struct drm_i915_private *dev_priv, unsigned long base,
 int i915_driver_load(struct drm_device *dev, unsigned long flags)
 {
 	struct drm_i915_private *dev_priv;
+	struct intel_device_info *info;
 	int ret = 0, mmio_bar;
 	uint32_t agp_size;
 
+	info = (struct intel_device_info *) flags;
+
+	/* Refuse to load on gen6+ without kms enabled. */
+	if (info->gen >= 6 && !drm_core_check_feature(dev, DRIVER_MODESET))
+		return -ENODEV;
+
+
 	/* i915 has 4 more counters */
 	dev->counters += 4;
 	dev->types[6] = _DRM_STAT_IRQ;
@@ -1971,7 +1979,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	dev->dev_private = (void *)dev_priv;
 	dev_priv->dev = dev;
-	dev_priv->info = (struct intel_device_info *) flags;
+	dev_priv->info = info;
 
 	if (i915_get_bridge_dev(dev)) {
 		ret = -EIO;
-- 
1.7.9

Re: [PATCH] drm/i915: refuse to load on gen6+ without kms

[Chris Wilson]

On Mon, 26 Mar 2012 21:33:18 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Spurred by an irc discussion, let's start to clear up which parts of
> our kms + ums/gem + ums/dri1 + vbios/dri1 kernel driver pieces
> userspace in the wild actually uses.
> 
> The idea is that we introduce checks at entry-points (module load
> time, ioctls, ...) first and then reap any obviously dead code in a
> second step.
> 
> As a first step refuse to load without kms on chips where userspace
> never supported ums. Now upstream hasn't supported ums on ilk, ever.
> But RHEL had the great idea to backport the kms support to their ums
> driver.
> 
> Cc: Dave Airlie <airlied@gmail.com>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_dma.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 64dfbb8..8567fdf 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1955,9 +1955,17 @@ i915_mtrr_setup(struct drm_i915_private *dev_priv, unsigned long base,
>  int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  {
>  	struct drm_i915_private *dev_priv;
> +	struct intel_device_info *info;
>  	int ret = 0, mmio_bar;
>  	uint32_t agp_size;
>  
> +	info = (struct intel_device_info *) flags;
> +
> +	/* Refuse to load on gen6+ without kms enabled. */
> +	if (info->gen >= 6 && !drm_core_check_feature(dev, DRIVER_MODESET))
> +		return -ENODEV;

The problem here is that this highlights that with the default
configuration we have no driver for our current hardware in the kernel.

You would also need to sneak

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index cc11488..ebc5135 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -139,6 +139,7 @@ config DRM_I915
 config DRM_I915_KMS
        bool "Enable modesetting on intel by default"
        depends on DRM_I915
+       default y
        help
          Choose this option if you want kernel modesetting enabled by default,
          and you have a new enough userspace to support this. Running old

past Linus first.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: refuse to load on gen6+ without kms

[Daniel Vetter]

On Mon, Mar 26, 2012 at 08:44:51PM +0100, Chris Wilson wrote:
> On Mon, 26 Mar 2012 21:33:18 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Spurred by an irc discussion, let's start to clear up which parts of
> > our kms + ums/gem + ums/dri1 + vbios/dri1 kernel driver pieces
> > userspace in the wild actually uses.
> > 
> > The idea is that we introduce checks at entry-points (module load
> > time, ioctls, ...) first and then reap any obviously dead code in a
> > second step.
> > 
> > As a first step refuse to load without kms on chips where userspace
> > never supported ums. Now upstream hasn't supported ums on ilk, ever.
> > But RHEL had the great idea to backport the kms support to their ums
> > driver.
> > 
> > Cc: Dave Airlie <airlied@gmail.com>
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c |   10 +++++++++-
> >  1 files changed, 9 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 64dfbb8..8567fdf 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1955,9 +1955,17 @@ i915_mtrr_setup(struct drm_i915_private *dev_priv, unsigned long base,
> >  int i915_driver_load(struct drm_device *dev, unsigned long flags)
> >  {
> >  	struct drm_i915_private *dev_priv;
> > +	struct intel_device_info *info;
> >  	int ret = 0, mmio_bar;
> >  	uint32_t agp_size;
> >  
> > +	info = (struct intel_device_info *) flags;
> > +
> > +	/* Refuse to load on gen6+ without kms enabled. */
> > +	if (info->gen >= 6 && !drm_core_check_feature(dev, DRIVER_MODESET))
> > +		return -ENODEV;
> 
> The problem here is that this highlights that with the default
> configuration we have no driver for our current hardware in the kernel.

Well, with the default configuration we _do_ have no driver for our
hardware. Non-kms doesn't do anything really on it's own, and without any
userspace existing no-one will call the entervt or gem_init ioctls.

No change at all imo.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: refuse to load on gen6+ without kms

[Adam Jackson]

[-- Attachment #1.1: Type: text/plain, Size: 687 bytes --]

On Mon, 2012-03-26 at 21:33 +0200, Daniel Vetter wrote:

> As a first step refuse to load without kms on chips where userspace
> never supported ums. Now upstream hasn't supported ums on ilk, ever.
> But RHEL had the great idea to backport the kms support to their ums
> driver.

In RHEL5, yes.  Personally I'm okay with saying those people get to run
RHEL5 kernels too, but Linus might have stronger opinions I guess.

That said we really don't use kernel support for this.  Certainly
there's no acceleration on ILK.  The corresponding kernel patch appears
to do nothing more than add the PCI IDs to the AGP driver, and add
IS_IRONLAKE() to I915_NEED_GFX_HWS().

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: disallow gem init ioctl on ilk

[Daniel Vetter]

Ums is already disabled, but on ilk we can additionally disable gem
initialization when using user mode setting. Upstream never support
ilk without kernel modesetting and not even the RHEL ilk ums backport
needs gem - that driver is based on xf86-video-intel version 2.2,
which is pre-gem.

Cc: Adam Jackson <ajax@redhat.com>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b704327..69d77b0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -152,6 +152,10 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data,
 	    (args->gtt_end | args->gtt_start) & (PAGE_SIZE - 1))
 		return -EINVAL;
 
+	/* GEM with user mode setting was never supported on ilk and later. */
+	if (INTEL_INFO(dev)->gen >= 5)
+		return -ENODEV;
+
 	mutex_lock(&dev->struct_mutex);
 	i915_gem_do_init(dev, args->gtt_start, args->gtt_end, args->gtt_end);
 	mutex_unlock(&dev->struct_mutex);
-- 
1.7.9

Re: [PATCH] drm/i915: disallow gem init ioctl on ilk

[Adam Jackson]

[-- Attachment #1.1: Type: text/plain, Size: 419 bytes --]

On Mon, 2012-03-26 at 22:37 +0200, Daniel Vetter wrote:
> Ums is already disabled, but on ilk we can additionally disable gem
> initialization when using user mode setting. Upstream never support
> ilk without kernel modesetting and not even the RHEL ilk ums backport
> needs gem - that driver is based on xf86-video-intel version 2.2,
> which is pre-gem.

Reviewed-by: Adam Jackson <ajax@redhat.com>

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disallow gem init ioctl on ilk

[Chris Wilson]

On Mon, 26 Mar 2012 22:37:04 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Ums is already disabled, but on ilk we can additionally disable gem
> initialization when using user mode setting. Upstream never support
> ilk without kernel modesetting and not even the RHEL ilk ums backport
> needs gem - that driver is based on xf86-video-intel version 2.2,
> which is pre-gem.
> 
> Cc: Adam Jackson <ajax@redhat.com>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: refuse to load on gen6+ without kms

[Chris Wilson]

On Mon, 26 Mar 2012 21:50:17 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Mar 26, 2012 at 08:44:51PM +0100, Chris Wilson wrote:
> > On Mon, 26 Mar 2012 21:33:18 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > Spurred by an irc discussion, let's start to clear up which parts of
> > > our kms + ums/gem + ums/dri1 + vbios/dri1 kernel driver pieces
> > > userspace in the wild actually uses.
> > > 
> > > The idea is that we introduce checks at entry-points (module load
> > > time, ioctls, ...) first and then reap any obviously dead code in a
> > > second step.
> > > 
> > > As a first step refuse to load without kms on chips where userspace
> > > never supported ums. Now upstream hasn't supported ums on ilk, ever.
> > > But RHEL had the great idea to backport the kms support to their ums
> > > driver.
> > > 
> > > Cc: Dave Airlie <airlied@gmail.com>
> > > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > ---
> > >  drivers/gpu/drm/i915/i915_dma.c |   10 +++++++++-
> > >  1 files changed, 9 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > > index 64dfbb8..8567fdf 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -1955,9 +1955,17 @@ i915_mtrr_setup(struct drm_i915_private *dev_priv, unsigned long base,
> > >  int i915_driver_load(struct drm_device *dev, unsigned long flags)
> > >  {
> > >  	struct drm_i915_private *dev_priv;
> > > +	struct intel_device_info *info;
> > >  	int ret = 0, mmio_bar;
> > >  	uint32_t agp_size;
> > >  
> > > +	info = (struct intel_device_info *) flags;
> > > +
> > > +	/* Refuse to load on gen6+ without kms enabled. */
> > > +	if (info->gen >= 6 && !drm_core_check_feature(dev, DRIVER_MODESET))
> > > +		return -ENODEV;
> > 
> > The problem here is that this highlights that with the default
> > configuration we have no driver for our current hardware in the kernel.
> 
> Well, with the default configuration we _do_ have no driver for our
> hardware. Non-kms doesn't do anything really on it's own, and without any
> userspace existing no-one will call the entervt or gem_init ioctls.
> 
> No change at all imo.

Just to make me happy, can you add a warning here that we are refusing
to load the driver because the user hasn't set a flag?
DRM_INFO("Not loading driver without KMS as this is an unsupported
configuration for this hardware; please enable CONFIG_DRM_I915_KMS or
pass i915.modeset=1 to proceed.\n");
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: disallow gem init ioctl on ilk

[Daniel Vetter]

On Mon, Mar 26, 2012 at 04:43:04PM -0400, Adam Jackson wrote:
> On Mon, 2012-03-26 at 22:37 +0200, Daniel Vetter wrote:
> > Ums is already disabled, but on ilk we can additionally disable gem
> > initialization when using user mode setting. Upstream never support
> > ilk without kernel modesetting and not even the RHEL ilk ums backport
> > needs gem - that driver is based on xf86-video-intel version 2.2,
> > which is pre-gem.
> 
> Reviewed-by: Adam Jackson <ajax@redhat.com>

I've picked this one and the previous one up for -next, thanks for the
review.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48