[PATCH 1/4] drm/i915: BIOS and power context stolen mem handling for VLV v7

[PATCH 1/4] drm/i915: BIOS and power context stolen mem handling for VLV v7

[Jesse Barnes]

But we need to get the right stolen base and make pre-allocated objects
for BIOS stuff so we don't clobber it.  If the BIOS hasn't allocated a
power context, we allocate one here too, from stolen space as required
by the docs.

v2: fix stolen to phys if ladder (Ben)
    keep BIOS reserved space out of allocator altogether (Ben)
v3: fix mask of stolen base (Ben)
v4: clean up preallocated object on unload (Ben)
    don't zero reg on unload (Jesse)
    fix mask harder (Jesse)
v5: use unref for freeing stolen bits (Chris)
    move alloc/free to intel_pm.c (Chris)
v6: NULL pctx at disable time so error paths work (Ben)
v7: use correct PCI device for config read (Jesse)

Reviewed-by: Ben Widawsky <benjamin.widawsky@intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h        |    2 ++
 drivers/gpu/drm/i915/i915_gem_stolen.c |   12 ++++++--
 drivers/gpu/drm/i915/i915_reg.h        |    1 +
 drivers/gpu/drm/i915/intel_pm.c        |   49 ++++++++++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c81100c..2fe5fd4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1073,6 +1073,8 @@ typedef struct drm_i915_private {
 
 	struct i915_gpu_error gpu_error;
 
+	struct drm_i915_gem_object *vlv_pctx;
+
 	/* list of fbdev register on this device */
 	struct intel_fbdev *fbdev;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 67d3510..913994c 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -62,7 +62,10 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 	 * its value of TOLUD.
 	 */
 	base = 0;
-	if (INTEL_INFO(dev)->gen >= 6) {
+	if (IS_VALLEYVIEW(dev)) {
+		pci_read_config_dword(dev->pdev, 0x5c, &base);
+		base &= ~((1<<20) - 1);
+	} else if (INTEL_INFO(dev)->gen >= 6) {
 		/* Read Base Data of Stolen Memory Register (BDSM) directly.
 		 * Note that there is also a MCHBAR miror at 0x1080c0 or
 		 * we could use device 2:0x5c instead.
@@ -183,6 +186,7 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
 int i915_gem_init_stolen(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	int bios_reserved = 0;
 
 	dev_priv->mm.stolen_base = i915_stolen_to_physical(dev);
 	if (dev_priv->mm.stolen_base == 0)
@@ -191,8 +195,12 @@ int i915_gem_init_stolen(struct drm_device *dev)
 	DRM_DEBUG_KMS("found %zd bytes of stolen memory at %08lx\n",
 		      dev_priv->gtt.stolen_size, dev_priv->mm.stolen_base);
 
+	if (IS_VALLEYVIEW(dev))
+		bios_reserved = 1024*1024; /* top 1M on VLV/BYT */
+
 	/* Basic memrange allocator for stolen space */
-	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size);
+	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
+		    bios_reserved);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a470103..a809a56 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -700,6 +700,7 @@
 #define VLV_IIR		(VLV_DISPLAY_BASE + 0x20a4)
 #define VLV_IMR		(VLV_DISPLAY_BASE + 0x20a8)
 #define VLV_ISR		(VLV_DISPLAY_BASE + 0x20ac)
+#define VLV_PCBR	(VLV_DISPLAY_BASE + 0x2120)
 #define   I915_PIPE_CONTROL_NOTIFY_INTERRUPT		(1<<18)
 #define   I915_DISPLAY_PORT_INTERRUPT			(1<<17)
 #define   I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9b3e90e..e60cd3e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2562,6 +2562,11 @@ static void valleyview_disable_rps(struct drm_device *dev)
 	spin_unlock_irq(&dev_priv->rps.lock);
 
 	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
+
+	if (dev_priv->vlv_pctx) {
+		drm_gem_object_unreference(&dev_priv->vlv_pctx->base);
+		dev_priv->vlv_pctx = NULL;
+	}
 }
 
 int intel_enable_rc6(const struct drm_device *dev)
@@ -2856,6 +2861,48 @@ static void vlv_rps_timer_work(struct work_struct *work)
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
+static void valleyview_setup_pctx(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *pctx;
+	unsigned long pctx_paddr;
+	u32 pcbr;
+	int pctx_size = 24*1024;
+
+	pcbr = I915_READ(VLV_PCBR);
+	if (pcbr) {
+		/* BIOS set it up already, grab the pre-alloc'd space */
+		int pcbr_offset;
+
+		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
+		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
+								      pcbr_offset,
+								      pcbr_offset,
+								      pctx_size);
+		goto out;
+	}
+
+	/*
+	 * From the Gunit register HAS:
+	 * The Gfx driver is expected to program this register and ensure
+	 * proper allocation within Gfx stolen memory.  For example, this
+	 * register should be programmed such than the PCBR range does not
+	 * overlap with other ranges, such as the frame buffer, protected
+	 * memory, or any other relevant ranges.
+	 */
+	pctx = i915_gem_object_create_stolen(dev, pctx_size);
+	if (!pctx) {
+		DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
+		return;
+	}
+
+	pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
+	I915_WRITE(VLV_PCBR, pctx_paddr);
+
+out:
+	dev_priv->vlv_pctx = pctx;
+}
+
 static void valleyview_enable_rps(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2870,6 +2917,8 @@ static void valleyview_enable_rps(struct drm_device *dev)
 		I915_WRITE(GTFIFODBG, gtfifodbg);
 	}
 
+	valleyview_setup_pctx(dev);
+
 	gen6_gt_force_wake_get(dev_priv);
 
 	I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);
-- 
1.7.10.4

[PATCH 2/4] drm/i915: allow stolen, pre-allocated objects to avoid GTT allocation v2

[Jesse Barnes]

In some cases, we may not need GTT address space allocated to a stolen
object, so allow passing -1 to the preallocated function to indicate as
much.

v2: remove BUG_ON(gtt_offset & 4095) now that -1 is allowed (Ville)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c |    5 ++++-
 drivers/gpu/drm/i915/intel_pm.c        |    2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 913994c..89cbfab 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -339,7 +339,6 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
 
 	/* KISS and expect everything to be page-aligned */
 	BUG_ON(stolen_offset & 4095);
-	BUG_ON(gtt_offset & 4095);
 	BUG_ON(size & 4095);
 
 	if (WARN_ON(size == 0))
@@ -360,6 +359,10 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
 		return NULL;
 	}
 
+	/* Some objects just need physical mem from stolen space */
+	if (gtt_offset == -1)
+		return obj;
+
 	/* To simplify the initialisation sequence between KMS and GTT,
 	 * we allow construction of the stolen object prior to
 	 * setting up the GTT space. The actual reservation will occur
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e60cd3e..081194d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2877,7 +2877,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
 		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
 		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
 								      pcbr_offset,
-								      pcbr_offset,
+								      -1,
 								      pctx_size);
 		goto out;
 	}
-- 
1.7.10.4

[PATCH 3/4] drm/i915: VLV support is no longer preliminary

[Jesse Barnes]

Works pretty well actually.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 624cdfc..e57b127 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -964,12 +964,6 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct intel_device_info *intel_info =
 		(struct intel_device_info *) ent->driver_data;
 
-	if (intel_info->is_valleyview)
-		if(!i915_preliminary_hw_support) {
-			DRM_ERROR("Preliminary hardware support disabled\n");
-			return -ENODEV;
-		}
-
 	/* Only bind to function 0 of the device. Early generations
 	 * used function 1 as a placeholder for multi-head. This causes
 	 * us confusion instead, especially on the systems where both
-- 
1.7.10.4

[PATCH 4/4] Revert "drm/i915: disable interrupts earlier in the driver unload code"

[Jesse Barnes]

This reverts commit fd0c06420d39958032655a04cfd194d5a7b38f83.
---
 drivers/gpu/drm/i915/intel_display.c |   19 +++++++------------
 drivers/gpu/drm/i915/intel_pm.c      |    3 ---
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d0c5ecf..e22e752 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9674,23 +9674,12 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	struct drm_crtc *crtc;
 	struct intel_crtc *intel_crtc;
 
-	/*
-	 * Interrupts and polling as the first thing to avoid creating havoc.
-	 * Too much stuff here (turning of rps, connectors, ...) would
-	 * experience fancy races otherwise.
-	 */
-	drm_irq_uninstall(dev);
-	cancel_work_sync(&dev_priv->hotplug_work);
-	/*
-	 * Due to the hpd irq storm handling the hotplug work can re-arm the
-	 * poll handlers. Hence disable polling after hpd handling is shut down.
-	 */
 	drm_kms_helper_poll_fini(dev);
-
 	mutex_lock(&dev->struct_mutex);
 
 	intel_unregister_dsm_handler();
 
+
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
 		/* Skip inactive CRTCs */
 		if (!crtc->fb)
@@ -9708,6 +9697,12 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	mutex_unlock(&dev->struct_mutex);
 
+	/* Disable the irq before mode object teardown, for the irq might
+	 * enqueue unpin/hotplug work. */
+	drm_irq_uninstall(dev);
+	cancel_work_sync(&dev_priv->hotplug_work);
+	cancel_work_sync(&dev_priv->rps.work);
+
 	/* flush any delayed tasks or pending work */
 	flush_scheduled_work();
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 081194d..992ff0d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3729,9 +3729,6 @@ void intel_disable_gt_powersave(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	/* Interrupts should be disabled already to avoid re-arming. */
-	WARN_ON(dev->irq_enabled);
-
 	if (IS_IRONLAKE_M(dev)) {
 		ironlake_disable_drps(dev);
 		ironlake_disable_rc6(dev);
-- 
1.7.10.4

Re: [PATCH 2/4] drm/i915: allow stolen, pre-allocated objects to avoid GTT allocation v2

[Ben Widawsky]

On Wed, May 08, 2013 at 10:45:14AM -0700, Jesse Barnes wrote:
> In some cases, we may not need GTT address space allocated to a stolen
> object, so allow passing -1 to the preallocated function to indicate as
> much.
> 
> v2: remove BUG_ON(gtt_offset & 4095) now that -1 is allowed (Ville)
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c |    5 ++++-
>  drivers/gpu/drm/i915/intel_pm.c        |    2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 913994c..89cbfab 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -339,7 +339,6 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
>  
>  	/* KISS and expect everything to be page-aligned */
>  	BUG_ON(stolen_offset & 4095);
> -	BUG_ON(gtt_offset & 4095);
>  	BUG_ON(size & 4095);
>  
>  	if (WARN_ON(size == 0))
> @@ -360,6 +359,10 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
>  		return NULL;
>  	}
>  
> +	/* Some objects just need physical mem from stolen space */
> +	if (gtt_offset == -1)
> +		return obj;
> +
>  	/* To simplify the initialisation sequence between KMS and GTT,
>  	 * we allow construction of the stolen object prior to
>  	 * setting up the GTT space. The actual reservation will occur
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e60cd3e..081194d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2877,7 +2877,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
>  		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
>  		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
>  								      pcbr_offset,
> -								      pcbr_offset,
> +								      -1,
>  								      pctx_size);
>  		goto out;
>  	}

I'm not asking for any changes, just want clarification.

Even the _i915_gem_object_create_stolen() call isn't very useful
in this case. You really don't even want a gem object in this case, just
to take it out of the stolen mm, so a simple call to drm_mm_create_block
would be sufficient.

In the case where we setup the powerctx ourselves, you really just need
to carve it out of the allocator too, because once created, we never
want to touch the thing... evicting it is out of the question as well.
Therefore, similar to the previous case, all you really need is
drm_mm_get_block().

Certainly for the second case, it's much cleaner to use the existing
APIs. It just showed up in my review that some of our object state
doesn't make sense for the power context (read/write domains, cache
level, has_dma_mapping, map_and_fenceable)

I'd be happy with a True/False for the first two paragraphs, and an
ignore on the third.

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 2/4] drm/i915: allow stolen, pre-allocated objects to avoid GTT allocation v2

[Jesse Barnes]

On Wed, 8 May 2013 19:12:49 -0700
Ben Widawsky <ben@bwidawsk.net> wrote:

> On Wed, May 08, 2013 at 10:45:14AM -0700, Jesse Barnes wrote:
> > In some cases, we may not need GTT address space allocated to a stolen
> > object, so allow passing -1 to the preallocated function to indicate as
> > much.
> > 
> > v2: remove BUG_ON(gtt_offset & 4095) now that -1 is allowed (Ville)
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_stolen.c |    5 ++++-
> >  drivers/gpu/drm/i915/intel_pm.c        |    2 +-
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > index 913994c..89cbfab 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -339,7 +339,6 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
> >  
> >  	/* KISS and expect everything to be page-aligned */
> >  	BUG_ON(stolen_offset & 4095);
> > -	BUG_ON(gtt_offset & 4095);
> >  	BUG_ON(size & 4095);
> >  
> >  	if (WARN_ON(size == 0))
> > @@ -360,6 +359,10 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
> >  		return NULL;
> >  	}
> >  
> > +	/* Some objects just need physical mem from stolen space */
> > +	if (gtt_offset == -1)
> > +		return obj;
> > +
> >  	/* To simplify the initialisation sequence between KMS and GTT,
> >  	 * we allow construction of the stolen object prior to
> >  	 * setting up the GTT space. The actual reservation will occur
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index e60cd3e..081194d 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2877,7 +2877,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
> >  		pcbr_offset = (pcbr & (~4095)) - dev_priv->mm.stolen_base;
> >  		pctx = i915_gem_object_create_stolen_for_preallocated(dev_priv->dev,
> >  								      pcbr_offset,
> > -								      pcbr_offset,
> > +								      -1,
> >  								      pctx_size);
> >  		goto out;
> >  	}
> 
> I'm not asking for any changes, just want clarification.
> 
> Even the _i915_gem_object_create_stolen() call isn't very useful
> in this case. You really don't even want a gem object in this case, just
> to take it out of the stolen mm, so a simple call to drm_mm_create_block
> would be sufficient.

Yeah, but outside of stolen.c I thought that would be a bit
underhanded.  We don't need the object really, but it doesn't hurt to
have it either, and it fits well for the case where we might need to
allocate it.

> In the case where we setup the powerctx ourselves, you really just need
> to carve it out of the allocator too, because once created, we never
> want to touch the thing... evicting it is out of the question as well.
> Therefore, similar to the previous case, all you really need is
> drm_mm_get_block().
> 
> Certainly for the second case, it's much cleaner to use the existing
> APIs. It just showed up in my review that some of our object state
> doesn't make sense for the power context (read/write domains, cache
> level, has_dma_mapping, map_and_fenceable)
> 
> I'd be happy with a True/False for the first two paragraphs, and an
> ignore on the third.

Yeah I'd be ok moving over to using something else for the power stuff
(and anything else that's a one time alloc, don't touch sort of
object), but it would be nicer if we had an API to do that, ideally
self-documenting.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 4/4] Revert "drm/i915: disable interrupts earlier in the driver unload code"

[Daniel Vetter]

On Wed, May 08, 2013 at 10:45:16AM -0700, Jesse Barnes wrote:
> This reverts commit fd0c06420d39958032655a04cfd194d5a7b38f83.

A bit thin on details about what exactly blows up ... can you please dig a
bit more?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |   19 +++++++------------
>  drivers/gpu/drm/i915/intel_pm.c      |    3 ---
>  2 files changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d0c5ecf..e22e752 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9674,23 +9674,12 @@ void intel_modeset_cleanup(struct drm_device *dev)
>  	struct drm_crtc *crtc;
>  	struct intel_crtc *intel_crtc;
>  
> -	/*
> -	 * Interrupts and polling as the first thing to avoid creating havoc.
> -	 * Too much stuff here (turning of rps, connectors, ...) would
> -	 * experience fancy races otherwise.
> -	 */
> -	drm_irq_uninstall(dev);
> -	cancel_work_sync(&dev_priv->hotplug_work);
> -	/*
> -	 * Due to the hpd irq storm handling the hotplug work can re-arm the
> -	 * poll handlers. Hence disable polling after hpd handling is shut down.
> -	 */
>  	drm_kms_helper_poll_fini(dev);
> -
>  	mutex_lock(&dev->struct_mutex);
>  
>  	intel_unregister_dsm_handler();
>  
> +
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>  		/* Skip inactive CRTCs */
>  		if (!crtc->fb)
> @@ -9708,6 +9697,12 @@ void intel_modeset_cleanup(struct drm_device *dev)
>  
>  	mutex_unlock(&dev->struct_mutex);
>  
> +	/* Disable the irq before mode object teardown, for the irq might
> +	 * enqueue unpin/hotplug work. */
> +	drm_irq_uninstall(dev);
> +	cancel_work_sync(&dev_priv->hotplug_work);
> +	cancel_work_sync(&dev_priv->rps.work);
> +
>  	/* flush any delayed tasks or pending work */
>  	flush_scheduled_work();
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 081194d..992ff0d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3729,9 +3729,6 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	/* Interrupts should be disabled already to avoid re-arming. */
> -	WARN_ON(dev->irq_enabled);
> -
>  	if (IS_IRONLAKE_M(dev)) {
>  		ironlake_disable_drps(dev);
>  		ironlake_disable_rc6(dev);
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/4] drm/i915: VLV support is no longer preliminary

[Daniel Vetter]

On Wed, May 08, 2013 at 10:45:15AM -0700, Jesse Barnes wrote:
> Works pretty well actually.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Patches 1-3 merged to dinq, thanks. I'm a bit unhappy that we don't have
mipi yet, otoh mipi needs sink drivers so meh.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/4] Revert "drm/i915: disable interrupts earlier in the driver unload code"

[Jesse Barnes]

On Fri, 10 May 2013 09:27:09 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, May 08, 2013 at 10:45:16AM -0700, Jesse Barnes wrote:
> > This reverts commit fd0c06420d39958032655a04cfd194d5a7b38f83.
> 
> A bit thin on details about what exactly blows up ... can you please dig a
> bit more?

On unload, we crash trying to queue the VLV work from the
gen6_pm_rps_work function and things fall over pretty hard.  At least
that's what the backtrace looked like; it was hard to tell since the
machine was hard hung and I couldn't scroll back.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 3/4] drm/i915: VLV support is no longer preliminary

[Jesse Barnes]

On Fri, 10 May 2013 09:29:28 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, May 08, 2013 at 10:45:15AM -0700, Jesse Barnes wrote:
> > Works pretty well actually.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Patches 1-3 merged to dinq, thanks. I'm a bit unhappy that we don't have
> mipi yet, otoh mipi needs sink drivers so meh.

Yeah, MIPI DSI is a bit messy regardless, so I think it's ok to call it
"ready" even if we don't have some specific DSI panels supported.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 4/4] Revert "drm/i915: disable interrupts earlier in the driver unload code"

[Jesse Barnes]

On Fri, 10 May 2013 09:27:09 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, May 08, 2013 at 10:45:16AM -0700, Jesse Barnes wrote:
> > This reverts commit fd0c06420d39958032655a04cfd194d5a7b38f83.
> 
> A bit thin on details about what exactly blows up ... can you please dig a
> bit more?

Oh and this wasn't an official submission :)  Just something to get you
thinking about it...

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 3/4] drm/i915: VLV support is no longer preliminary

[Daniel Vetter]

On Fri, May 10, 2013 at 09:10:27AM -0700, Jesse Barnes wrote:
> On Fri, 10 May 2013 09:29:28 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Wed, May 08, 2013 at 10:45:15AM -0700, Jesse Barnes wrote:
> > > Works pretty well actually.
> > > 
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > Patches 1-3 merged to dinq, thanks. I'm a bit unhappy that we don't have
> > mipi yet, otoh mipi needs sink drivers so meh.
> 
> Yeah, MIPI DSI is a bit messy regardless, so I think it's ok to call it
> "ready" even if we don't have some specific DSI panels supported.

Another thing which looked _really_ fishy was non-eDP DP support on vlv.
Atm we often only take the new vlv dp paths due to is_cpu_edp being true
on vlv edp, too. Imo that should be remedied and we should move all the
vlv paths out of the is_cpu_edp checks (and maybe inline the port ==
PORT_A check for ilk-ivb in all relevant places). Imre has ideas already,
too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/4] drm/i915: VLV support is no longer preliminary

[Jesse Barnes]

On Fri, 10 May 2013 20:58:17 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, May 10, 2013 at 09:10:27AM -0700, Jesse Barnes wrote:
> > On Fri, 10 May 2013 09:29:28 +0200
> > Daniel Vetter <daniel@ffwll.ch> wrote:
> > 
> > > On Wed, May 08, 2013 at 10:45:15AM -0700, Jesse Barnes wrote:
> > > > Works pretty well actually.
> > > > 
> > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > 
> > > Patches 1-3 merged to dinq, thanks. I'm a bit unhappy that we don't have
> > > mipi yet, otoh mipi needs sink drivers so meh.
> > 
> > Yeah, MIPI DSI is a bit messy regardless, so I think it's ok to call it
> > "ready" even if we don't have some specific DSI panels supported.
> 
> Another thing which looked _really_ fishy was non-eDP DP support on vlv.
> Atm we often only take the new vlv dp paths due to is_cpu_edp being true
> on vlv edp, too. Imo that should be remedied and we should move all the
> vlv paths out of the is_cpu_edp checks (and maybe inline the port ==
> PORT_A check for ilk-ivb in all relevant places). Imre has ideas already,
> too.

Ok just be careful, because regular DP works fine here in testing. :)

-- 
Jesse Barnes, Intel Open Source Technology Center