[PATCH 0/6] drm/i915: Implement gen3/4 GPU reset

[PATCH 0/6] drm/i915: Implement gen3/4 GPU reset

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

This is a respin of my earlier gen3/4 GPU reset patches. Ken was complaining
about the lack of reset on gen4 and I pointed him at my branch, which he used
with some success.

I had to do a bit of rebasing on the patches, but I left Ken's tested-by in
place. Originally we had irq disable/enable in the common codepath but that
has since disappeared so I added something for that in the gen3/4 codepath.
Works for me so I think the tested-by is still valid.

The new thing with this series is the whole modeset_lock stuff, which cures
a lot of dmesg warnings from the modeset code. Deadlocks have historically
been a problem with this stuff. I'm hoping I didn't introduce any new ones,
but this could certainly use someone else thinking through it as well.

My 946GZ seems quite happy with this. Only some spurious crc irq warnings and
a few vblank sequence number oddities left when I rung through the igt
reset/hang tests. I didn't spend any time really looking at those. I was sort
of hoping the crtc_disable thing might have fixed the vblank stuff, but no
such luck.

Pushed here:
git://gitorious.org/vsyrjala/linux.git gpu_reset_fixes_7

Ville Syrjälä (6):
  drm/i915: Fix gen4 GPU reset
  drm/i915: Restore the display config after a GPU reset on gen4
  drm/i915: Implement GPU reset for 915/945
  drm/i915: Implement GPU reset for g33
  drm/i915: Grab modeset locks for GPU rest on pre-ctg
  drm/i915: Disable crtcs gracefully before GPU reset on gen3/4

 drivers/gpu/drm/i915/i915_irq.c      |   5 +-
 drivers/gpu/drm/i915/i915_reg.h      |   3 +-
 drivers/gpu/drm/i915/intel_display.c | 108 +++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h     |   3 +-
 drivers/gpu/drm/i915/intel_uncore.c  |  63 ++++++++++----------
 5 files changed, 129 insertions(+), 53 deletions(-)

-- 
2.0.4

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

[PATCH 1/6] drm/i915: Fix gen4 GPU reset

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On pre-ctg the reset bit directly controls the reset signal. We must
assert it for >=20usec and then deassert it. Bit 1 is a RO status bit
which should also go down when the reset is no longer asserted.

Tested-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h     |  1 +
 drivers/gpu/drm/i915/intel_uncore.c | 41 +++++++++++++------------------------
 2 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3102907..ff1e36f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -83,6 +83,7 @@
 #define  GRDOM_RENDER	(1<<2)
 #define  GRDOM_MEDIA	(3<<2)
 #define  GRDOM_MASK	(3<<2)
+#define  GRDOM_RESET_STATUS (1<<1)
 #define  GRDOM_RESET_ENABLE (1<<0)
 
 #define ILK_GDSR 0x2ca4 /* MCHBAR offset */
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 1a3e485..6516fa1 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1349,37 +1349,24 @@ static int i965_reset_complete(struct drm_device *dev)
 {
 	u8 gdrst;
 	pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
-	return (gdrst & GRDOM_RESET_ENABLE) == 0;
+	return (gdrst & GRDOM_RESET_STATUS) == 0;
 }
 
 static int i965_do_reset(struct drm_device *dev)
 {
-	int ret;
-
-	/* FIXME: i965g/gm need a display save/restore for gpu reset. */
-	return -ENODEV;
-
-	/*
-	 * Set the domains we want to reset (GRDOM/bits 2 and 3) as
-	 * well as the reset bit (GR/bit 0).  Setting the GR bit
-	 * triggers the reset; when done, the hardware will clear it.
-	 */
-	pci_write_config_byte(dev->pdev, I965_GDRST,
-			      GRDOM_RENDER | GRDOM_RESET_ENABLE);
-	ret =  wait_for(i965_reset_complete(dev), 500);
-	if (ret)
-		return ret;
-
-	pci_write_config_byte(dev->pdev, I965_GDRST,
-			      GRDOM_MEDIA | GRDOM_RESET_ENABLE);
-
-	ret =  wait_for(i965_reset_complete(dev), 500);
-	if (ret)
-		return ret;
-
+	/* assert reset for at least 20 usec */
+	pci_write_config_byte(dev->pdev, I965_GDRST, GRDOM_RESET_ENABLE);
+	udelay(20);
 	pci_write_config_byte(dev->pdev, I965_GDRST, 0);
 
-	return 0;
+	return wait_for(i965_reset_complete(dev), 500);
+}
+
+static int g4x_reset_complete(struct drm_device *dev)
+{
+	u8 gdrst;
+	pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
+	return (gdrst & GRDOM_RESET_ENABLE) == 0;
 }
 
 static int g4x_do_reset(struct drm_device *dev)
@@ -1389,7 +1376,7 @@ static int g4x_do_reset(struct drm_device *dev)
 
 	pci_write_config_byte(dev->pdev, I965_GDRST,
 			      GRDOM_RENDER | GRDOM_RESET_ENABLE);
-	ret =  wait_for(i965_reset_complete(dev), 500);
+	ret =  wait_for(g4x_reset_complete(dev), 500);
 	if (ret)
 		return ret;
 
@@ -1399,7 +1386,7 @@ static int g4x_do_reset(struct drm_device *dev)
 
 	pci_write_config_byte(dev->pdev, I965_GDRST,
 			      GRDOM_MEDIA | GRDOM_RESET_ENABLE);
-	ret =  wait_for(i965_reset_complete(dev), 500);
+	ret =  wait_for(g4x_reset_complete(dev), 500);
 	if (ret)
 		return ret;
 
-- 
2.0.4

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

Re: [PATCH 1/6] drm/i915: Fix gen4 GPU reset

[Chris Wilson]

On Fri, Nov 21, 2014 at 09:54:25PM +0200, ville.syrjala@linux.intel.com wrote:
> +	/* assert reset for at least 20 usec */
> +	pci_write_config_byte(dev->pdev, I965_GDRST, GRDOM_RESET_ENABLE);

Is this an UC write or do we need to post?

> +	udelay(20);
>  	pci_write_config_byte(dev->pdev, I965_GDRST, 0);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: Fix gen4 GPU reset

[Ville Syrjälä]

On Sat, Nov 22, 2014 at 11:05:15AM +0000, Chris Wilson wrote:
> On Fri, Nov 21, 2014 at 09:54:25PM +0200, ville.syrjala@linux.intel.com wrote:
> > +	/* assert reset for at least 20 usec */
> > +	pci_write_config_byte(dev->pdev, I965_GDRST, GRDOM_RESET_ENABLE);
> 
> Is this an UC write or do we need to post?

I believe config space writes should always be be non-posted.

> 
> > +	udelay(20);
> >  	pci_write_config_byte(dev->pdev, I965_GDRST, 0);
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/6] drm/i915: Restore the display config after a GPU reset on gen4

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On pre-ctg GPU reset also resets the display hardware. Force a mode
restore after the GPU reset, and also re-init clock gating.

v2: Use intel_modeset_init_hw() instead of intel_init_clock_gating()
    in case more relevant stuff gets added there at some point
    Restore interrupts after the reset as well

Tested-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1e9c136..68e4239 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -880,6 +880,24 @@ int i915_reset(struct drm_device *dev)
 		 */
 		if (INTEL_INFO(dev)->gen > 5)
 			intel_reset_gt_powersave(dev);
+
+		if (IS_GEN4(dev) && !IS_G4X(dev)) {
+			intel_runtime_pm_disable_interrupts(dev_priv);
+			intel_runtime_pm_enable_interrupts(dev_priv);
+
+			intel_modeset_init_hw(dev);
+
+			spin_lock_irq(&dev_priv->irq_lock);
+			if (dev_priv->display.hpd_irq_setup)
+				dev_priv->display.hpd_irq_setup(dev);
+			spin_unlock_irq(&dev_priv->irq_lock);
+
+			drm_modeset_lock_all(dev);
+			intel_modeset_setup_hw_state(dev, true);
+			drm_modeset_unlock_all(dev);
+
+			intel_hpd_init(dev_priv);
+		}
 	} else {
 		mutex_unlock(&dev->struct_mutex);
 	}
-- 
2.0.4

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

[PATCH 3/6] drm/i915: Implement GPU reset for 915/945

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

915/945 have the same reset registers as 965, so share the code.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c     |  3 ++-
 drivers/gpu/drm/i915/i915_reg.h     |  2 +-
 drivers/gpu/drm/i915/intel_uncore.c | 24 ++++++++++++------------
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 68e4239..44abd7b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -881,7 +881,8 @@ int i915_reset(struct drm_device *dev)
 		if (INTEL_INFO(dev)->gen > 5)
 			intel_reset_gt_powersave(dev);
 
-		if (IS_GEN4(dev) && !IS_G4X(dev)) {
+		if ((IS_GEN3(dev) && !IS_G33(dev)) ||
+		    (IS_GEN4(dev) && !IS_G4X(dev))) {
 			intel_runtime_pm_disable_interrupts(dev_priv);
 			intel_runtime_pm_enable_interrupts(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ff1e36f..5446758 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -78,7 +78,7 @@
 
 
 /* Graphics reset regs */
-#define I965_GDRST 0xc0 /* PCI config register */
+#define I915_GDRST 0xc0 /* PCI config register */
 #define  GRDOM_FULL	(0<<2)
 #define  GRDOM_RENDER	(1<<2)
 #define  GRDOM_MEDIA	(3<<2)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 6516fa1..54f8265 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1345,27 +1345,27 @@ int i915_get_reset_stats_ioctl(struct drm_device *dev,
 	return 0;
 }
 
-static int i965_reset_complete(struct drm_device *dev)
+static int i915_reset_complete(struct drm_device *dev)
 {
 	u8 gdrst;
-	pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
+	pci_read_config_byte(dev->pdev, I915_GDRST, &gdrst);
 	return (gdrst & GRDOM_RESET_STATUS) == 0;
 }
 
-static int i965_do_reset(struct drm_device *dev)
+static int i915_do_reset(struct drm_device *dev)
 {
 	/* assert reset for at least 20 usec */
-	pci_write_config_byte(dev->pdev, I965_GDRST, GRDOM_RESET_ENABLE);
+	pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
 	udelay(20);
-	pci_write_config_byte(dev->pdev, I965_GDRST, 0);
+	pci_write_config_byte(dev->pdev, I915_GDRST, 0);
 
-	return wait_for(i965_reset_complete(dev), 500);
+	return wait_for(i915_reset_complete(dev), 500);
 }
 
 static int g4x_reset_complete(struct drm_device *dev)
 {
 	u8 gdrst;
-	pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
+	pci_read_config_byte(dev->pdev, I915_GDRST, &gdrst);
 	return (gdrst & GRDOM_RESET_ENABLE) == 0;
 }
 
@@ -1374,7 +1374,7 @@ static int g4x_do_reset(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
-	pci_write_config_byte(dev->pdev, I965_GDRST,
+	pci_write_config_byte(dev->pdev, I915_GDRST,
 			      GRDOM_RENDER | GRDOM_RESET_ENABLE);
 	ret =  wait_for(g4x_reset_complete(dev), 500);
 	if (ret)
@@ -1384,7 +1384,7 @@ static int g4x_do_reset(struct drm_device *dev)
 	I915_WRITE(VDECCLK_GATE_D, I915_READ(VDECCLK_GATE_D) | VCP_UNIT_CLOCK_GATE_DISABLE);
 	POSTING_READ(VDECCLK_GATE_D);
 
-	pci_write_config_byte(dev->pdev, I965_GDRST,
+	pci_write_config_byte(dev->pdev, I915_GDRST,
 			      GRDOM_MEDIA | GRDOM_RESET_ENABLE);
 	ret =  wait_for(g4x_reset_complete(dev), 500);
 	if (ret)
@@ -1394,7 +1394,7 @@ static int g4x_do_reset(struct drm_device *dev)
 	I915_WRITE(VDECCLK_GATE_D, I915_READ(VDECCLK_GATE_D) & ~VCP_UNIT_CLOCK_GATE_DISABLE);
 	POSTING_READ(VDECCLK_GATE_D);
 
-	pci_write_config_byte(dev->pdev, I965_GDRST, 0);
+	pci_write_config_byte(dev->pdev, I915_GDRST, 0);
 
 	return 0;
 }
@@ -1452,8 +1452,8 @@ int intel_gpu_reset(struct drm_device *dev)
 		return ironlake_do_reset(dev);
 	else if (IS_G4X(dev))
 		return g4x_do_reset(dev);
-	else if (IS_GEN4(dev))
-		return i965_do_reset(dev);
+	else if (IS_GEN4(dev) || (IS_GEN3(dev) &&  !IS_G33(dev)))
+		return i915_do_reset(dev);
 	else
 		return -ENODEV;
 }
-- 
2.0.4

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

Re: [PATCH 3/6] drm/i915: Implement GPU reset for 915/945

[Daniel Vetter]

On Fri, Nov 21, 2014 at 09:54:27PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 915/945 have the same reset registers as 965, so share the code.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I didn't find the gmch docs anywhere for these. Care to share?

Anyway, merged the first 6 patches from this series to dinq, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.c     |  3 ++-
>  drivers/gpu/drm/i915/i915_reg.h     |  2 +-
>  drivers/gpu/drm/i915/intel_uncore.c | 24 ++++++++++++------------
>  3 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 68e4239..44abd7b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -881,7 +881,8 @@ int i915_reset(struct drm_device *dev)
>  		if (INTEL_INFO(dev)->gen > 5)
>  			intel_reset_gt_powersave(dev);
>  
> -		if (IS_GEN4(dev) && !IS_G4X(dev)) {
> +		if ((IS_GEN3(dev) && !IS_G33(dev)) ||
> +		    (IS_GEN4(dev) && !IS_G4X(dev))) {
>  			intel_runtime_pm_disable_interrupts(dev_priv);
>  			intel_runtime_pm_enable_interrupts(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ff1e36f..5446758 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -78,7 +78,7 @@
>  
>  
>  /* Graphics reset regs */
> -#define I965_GDRST 0xc0 /* PCI config register */
> +#define I915_GDRST 0xc0 /* PCI config register */
>  #define  GRDOM_FULL	(0<<2)
>  #define  GRDOM_RENDER	(1<<2)
>  #define  GRDOM_MEDIA	(3<<2)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 6516fa1..54f8265 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1345,27 +1345,27 @@ int i915_get_reset_stats_ioctl(struct drm_device *dev,
>  	return 0;
>  }
>  
> -static int i965_reset_complete(struct drm_device *dev)
> +static int i915_reset_complete(struct drm_device *dev)
>  {
>  	u8 gdrst;
> -	pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
> +	pci_read_config_byte(dev->pdev, I915_GDRST, &gdrst);
>  	return (gdrst & GRDOM_RESET_STATUS) == 0;
>  }
>  
> -static int i965_do_reset(struct drm_device *dev)
> +static int i915_do_reset(struct drm_device *dev)
>  {
>  	/* assert reset for at least 20 usec */
> -	pci_write_config_byte(dev->pdev, I965_GDRST, GRDOM_RESET_ENABLE);
> +	pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
>  	udelay(20);
> -	pci_write_config_byte(dev->pdev, I965_GDRST, 0);
> +	pci_write_config_byte(dev->pdev, I915_GDRST, 0);
>  
> -	return wait_for(i965_reset_complete(dev), 500);
> +	return wait_for(i915_reset_complete(dev), 500);
>  }
>  
>  static int g4x_reset_complete(struct drm_device *dev)
>  {
>  	u8 gdrst;
> -	pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst);
> +	pci_read_config_byte(dev->pdev, I915_GDRST, &gdrst);
>  	return (gdrst & GRDOM_RESET_ENABLE) == 0;
>  }
>  
> @@ -1374,7 +1374,7 @@ static int g4x_do_reset(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
>  
> -	pci_write_config_byte(dev->pdev, I965_GDRST,
> +	pci_write_config_byte(dev->pdev, I915_GDRST,
>  			      GRDOM_RENDER | GRDOM_RESET_ENABLE);
>  	ret =  wait_for(g4x_reset_complete(dev), 500);
>  	if (ret)
> @@ -1384,7 +1384,7 @@ static int g4x_do_reset(struct drm_device *dev)
>  	I915_WRITE(VDECCLK_GATE_D, I915_READ(VDECCLK_GATE_D) | VCP_UNIT_CLOCK_GATE_DISABLE);
>  	POSTING_READ(VDECCLK_GATE_D);
>  
> -	pci_write_config_byte(dev->pdev, I965_GDRST,
> +	pci_write_config_byte(dev->pdev, I915_GDRST,
>  			      GRDOM_MEDIA | GRDOM_RESET_ENABLE);
>  	ret =  wait_for(g4x_reset_complete(dev), 500);
>  	if (ret)
> @@ -1394,7 +1394,7 @@ static int g4x_do_reset(struct drm_device *dev)
>  	I915_WRITE(VDECCLK_GATE_D, I915_READ(VDECCLK_GATE_D) & ~VCP_UNIT_CLOCK_GATE_DISABLE);
>  	POSTING_READ(VDECCLK_GATE_D);
>  
> -	pci_write_config_byte(dev->pdev, I965_GDRST, 0);
> +	pci_write_config_byte(dev->pdev, I915_GDRST, 0);
>  
>  	return 0;
>  }
> @@ -1452,8 +1452,8 @@ int intel_gpu_reset(struct drm_device *dev)
>  		return ironlake_do_reset(dev);
>  	else if (IS_G4X(dev))
>  		return g4x_do_reset(dev);
> -	else if (IS_GEN4(dev))
> -		return i965_do_reset(dev);
> +	else if (IS_GEN4(dev) || (IS_GEN3(dev) &&  !IS_G33(dev)))
> +		return i915_do_reset(dev);
>  	else
>  		return -ENODEV;
>  }
> -- 
> 2.0.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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/6] drm/i915: Implement GPU reset for g33

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

g33 seems to sit somewhere between the 915/945/965 style and the
g4x style. The bits look like g4x, but we still need to do a full
reset including display.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c     |  4 ++--
 drivers/gpu/drm/i915/intel_uncore.c | 12 +++++++++++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 44abd7b..5066fd1 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -881,8 +881,8 @@ int i915_reset(struct drm_device *dev)
 		if (INTEL_INFO(dev)->gen > 5)
 			intel_reset_gt_powersave(dev);
 
-		if ((IS_GEN3(dev) && !IS_G33(dev)) ||
-		    (IS_GEN4(dev) && !IS_G4X(dev))) {
+
+		if (IS_GEN3(dev) || (IS_GEN4(dev) && !IS_G4X(dev))) {
 			intel_runtime_pm_disable_interrupts(dev_priv);
 			intel_runtime_pm_enable_interrupts(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 54f8265..0f69369 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1369,6 +1369,14 @@ static int g4x_reset_complete(struct drm_device *dev)
 	return (gdrst & GRDOM_RESET_ENABLE) == 0;
 }
 
+static int g33_do_reset(struct drm_device *dev)
+{
+	/* FIXME spec says to turn off all planes and wait 1 usec before reset */
+
+	pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
+	return wait_for(g4x_reset_complete(dev), 500);
+}
+
 static int g4x_do_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1452,7 +1460,9 @@ int intel_gpu_reset(struct drm_device *dev)
 		return ironlake_do_reset(dev);
 	else if (IS_G4X(dev))
 		return g4x_do_reset(dev);
-	else if (IS_GEN4(dev) || (IS_GEN3(dev) &&  !IS_G33(dev)))
+	else if (IS_G33(dev))
+		return g33_do_reset(dev);
+	else if (INTEL_INFO(dev)->gen >= 3)
 		return i915_do_reset(dev);
 	else
 		return -ENODEV;
-- 
2.0.4

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

[PATCH 5/6] drm/i915: Grab modeset locks for GPU rest on pre-ctg

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On gen4 and earlier the GPU reset also resets the display, so we should
protect against concurrent modeset operations. Grab all the modeset locks
around the entire GPU reset dance, remebering first ti dislogde any
pending page flip to make sure we don't deadlock. Any pageflip coming
in between these two steps should fail anyway due to reset_in_progress,
so this should be safe.

This fixes a lot of failed asserts in the modeset code when there's a
modeset racing with the reset. Naturally the asserts aren't happy when
the expected state has disappeared.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      | 19 -------
 drivers/gpu/drm/i915/i915_irq.c      |  5 +-
 drivers/gpu/drm/i915/intel_display.c | 96 ++++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +-
 4 files changed, 86 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5066fd1..1e9c136 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -880,25 +880,6 @@ int i915_reset(struct drm_device *dev)
 		 */
 		if (INTEL_INFO(dev)->gen > 5)
 			intel_reset_gt_powersave(dev);
-
-
-		if (IS_GEN3(dev) || (IS_GEN4(dev) && !IS_G4X(dev))) {
-			intel_runtime_pm_disable_interrupts(dev_priv);
-			intel_runtime_pm_enable_interrupts(dev_priv);
-
-			intel_modeset_init_hw(dev);
-
-			spin_lock_irq(&dev_priv->irq_lock);
-			if (dev_priv->display.hpd_irq_setup)
-				dev_priv->display.hpd_irq_setup(dev);
-			spin_unlock_irq(&dev_priv->irq_lock);
-
-			drm_modeset_lock_all(dev);
-			intel_modeset_setup_hw_state(dev, true);
-			drm_modeset_unlock_all(dev);
-
-			intel_hpd_init(dev_priv);
-		}
 	} else {
 		mutex_unlock(&dev->struct_mutex);
 	}
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5908580d..8887674 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2428,6 +2428,9 @@ static void i915_error_work_func(struct work_struct *work)
 		 * simulated reset via debugs, so get an RPM reference.
 		 */
 		intel_runtime_pm_get(dev_priv);
+
+		intel_prepare_reset(dev);
+
 		/*
 		 * All state reset _must_ be completed before we update the
 		 * reset counter, for otherwise waiters might miss the reset
@@ -2436,7 +2439,7 @@ static void i915_error_work_func(struct work_struct *work)
 		 */
 		ret = i915_reset(dev);
 
-		intel_display_handle_reset(dev);
+		intel_finish_reset(dev);
 
 		intel_runtime_pm_put(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3218455..8329f7c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2765,25 +2765,10 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	return 0;
 }
 
-void intel_display_handle_reset(struct drm_device *dev)
+static void intel_complete_page_flips(struct drm_device *dev)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
 
-	/*
-	 * Flips in the rings have been nuked by the reset,
-	 * so complete all pending flips so that user space
-	 * will get its events and not get stuck.
-	 *
-	 * Also update the base address of all primary
-	 * planes to the the last fb to make sure we're
-	 * showing the correct fb after a reset.
-	 *
-	 * Need to make two loops over the crtcs so that we
-	 * don't try to grab a crtc mutex before the
-	 * pending_flip_queue really got woken up.
-	 */
-
 	for_each_crtc(dev, crtc) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		enum plane plane = intel_crtc->plane;
@@ -2791,6 +2776,12 @@ void intel_display_handle_reset(struct drm_device *dev)
 		intel_prepare_page_flip(dev, plane);
 		intel_finish_page_flip_plane(dev, plane);
 	}
+}
+
+static void intel_update_primary_planes(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
 
 	for_each_crtc(dev, crtc) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -2810,6 +2801,79 @@ void intel_display_handle_reset(struct drm_device *dev)
 	}
 }
 
+void intel_prepare_reset(struct drm_device *dev)
+{
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return;
+
+	/*
+	 * Flips in the rings will be nuked by the reset,
+	 * so complete all pending flips so that user space
+	 * will get its events and not get stuck.
+	 *
+	 * Old platforms will also reset the display, so we
+	 * need to grab the modeset locks around the reset.
+	 * But in order to do that we must let any pending
+	 * page flip wait complete since the waiters may be
+	 * holding some modeset locks.
+	 */
+	intel_complete_page_flips(dev);
+
+	/* no reset support for gen2 */
+	if (IS_GEN2(dev))
+		return;
+
+	/* reset doesn't touch the display */
+	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
+		return;
+
+	drm_modeset_lock_all(dev);
+}
+
+void intel_finish_reset(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return;
+
+	/* no reset support for gen2 */
+	if (IS_GEN2(dev))
+		return;
+
+	/* reset doesn't touch the display */
+	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
+		/*
+		 * Flips in the rings have been nuked by the reset,
+		 * so update the base address of all primary
+		 * planes to the the last fb to make sure we're
+		 * showing the correct fb after a reset.
+		 */
+		intel_update_primary_planes(dev);
+		return;
+	}
+
+	/*
+	 * The display has been reset as well,
+	 * so need a full re-initialization.
+	 */
+	intel_runtime_pm_disable_interrupts(dev_priv);
+	intel_runtime_pm_enable_interrupts(dev_priv);
+
+	intel_modeset_init_hw(dev);
+
+	spin_lock_irq(&dev_priv->irq_lock);
+	if (dev_priv->display.hpd_irq_setup)
+		dev_priv->display.hpd_irq_setup(dev);
+	spin_unlock_irq(&dev_priv->irq_lock);
+
+	intel_modeset_setup_hw_state(dev, true);
+
+	intel_hpd_init(dev_priv);
+
+	drm_modeset_unlock_all(dev);
+}
+
 static int
 intel_finish_fb(struct drm_framebuffer *old_fb)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f0a46ec..25fdbb1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -958,7 +958,8 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 					     unsigned int tiling_mode,
 					     unsigned int bpp,
 					     unsigned int pitch);
-void intel_display_handle_reset(struct drm_device *dev);
+void intel_prepare_reset(struct drm_device *dev);
+void intel_finish_reset(struct drm_device *dev);
 void hsw_enable_pc8(struct drm_i915_private *dev_priv);
 void hsw_disable_pc8(struct drm_i915_private *dev_priv);
 void intel_dp_get_m_n(struct intel_crtc *crtc,
-- 
2.0.4

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

Re: [PATCH 5/6] drm/i915: Grab modeset locks for GPU rest on pre-ctg

[Daniel Vetter]

On Fri, Nov 21, 2014 at 09:54:29PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On gen4 and earlier the GPU reset also resets the display, so we should
> protect against concurrent modeset operations. Grab all the modeset locks
> around the entire GPU reset dance, remebering first ti dislogde any
> pending page flip to make sure we don't deadlock. Any pageflip coming
> in between these two steps should fail anyway due to reset_in_progress,
> so this should be safe.
> 
> This fixes a lot of failed asserts in the modeset code when there's a
> modeset racing with the reset. Naturally the asserts aren't happy when
> the expected state has disappeared.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Two comments on this one, otherwise looks good (well didn't bother to
check the new reset register frobbing).
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.c      | 19 -------
>  drivers/gpu/drm/i915/i915_irq.c      |  5 +-
>  drivers/gpu/drm/i915/intel_display.c | 96 ++++++++++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
>  4 files changed, 86 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 5066fd1..1e9c136 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -880,25 +880,6 @@ int i915_reset(struct drm_device *dev)
>  		 */
>  		if (INTEL_INFO(dev)->gen > 5)
>  			intel_reset_gt_powersave(dev);
> -
> -
> -		if (IS_GEN3(dev) || (IS_GEN4(dev) && !IS_G4X(dev))) {
> -			intel_runtime_pm_disable_interrupts(dev_priv);
> -			intel_runtime_pm_enable_interrupts(dev_priv);
> -
> -			intel_modeset_init_hw(dev);
> -
> -			spin_lock_irq(&dev_priv->irq_lock);
> -			if (dev_priv->display.hpd_irq_setup)
> -				dev_priv->display.hpd_irq_setup(dev);
> -			spin_unlock_irq(&dev_priv->irq_lock);
> -
> -			drm_modeset_lock_all(dev);
> -			intel_modeset_setup_hw_state(dev, true);
> -			drm_modeset_unlock_all(dev);
> -
> -			intel_hpd_init(dev_priv);
> -		}
>  	} else {
>  		mutex_unlock(&dev->struct_mutex);
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 5908580d..8887674 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2428,6 +2428,9 @@ static void i915_error_work_func(struct work_struct *work)
>  		 * simulated reset via debugs, so get an RPM reference.
>  		 */
>  		intel_runtime_pm_get(dev_priv);
> +
> +		intel_prepare_reset(dev);
> +
>  		/*
>  		 * All state reset _must_ be completed before we update the
>  		 * reset counter, for otherwise waiters might miss the reset
> @@ -2436,7 +2439,7 @@ static void i915_error_work_func(struct work_struct *work)
>  		 */
>  		ret = i915_reset(dev);
>  
> -		intel_display_handle_reset(dev);
> +		intel_finish_reset(dev);
>  
>  		intel_runtime_pm_put(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3218455..8329f7c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2765,25 +2765,10 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	return 0;
>  }
>  
> -void intel_display_handle_reset(struct drm_device *dev)
> +static void intel_complete_page_flips(struct drm_device *dev)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *crtc;
>  
> -	/*
> -	 * Flips in the rings have been nuked by the reset,
> -	 * so complete all pending flips so that user space
> -	 * will get its events and not get stuck.
> -	 *
> -	 * Also update the base address of all primary
> -	 * planes to the the last fb to make sure we're
> -	 * showing the correct fb after a reset.
> -	 *
> -	 * Need to make two loops over the crtcs so that we
> -	 * don't try to grab a crtc mutex before the
> -	 * pending_flip_queue really got woken up.
> -	 */
> -
>  	for_each_crtc(dev, crtc) {
>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  		enum plane plane = intel_crtc->plane;
> @@ -2791,6 +2776,12 @@ void intel_display_handle_reset(struct drm_device *dev)
>  		intel_prepare_page_flip(dev, plane);
>  		intel_finish_page_flip_plane(dev, plane);
>  	}
> +}
> +
> +static void intel_update_primary_planes(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
>  
>  	for_each_crtc(dev, crtc) {
>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -2810,6 +2801,79 @@ void intel_display_handle_reset(struct drm_device *dev)
>  	}
>  }
>  
> +void intel_prepare_reset(struct drm_device *dev)
> +{
> +	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +		return;

ums just officially died please remove. Same for the one below.

> +
> +	/*
> +	 * Flips in the rings will be nuked by the reset,
> +	 * so complete all pending flips so that user space
> +	 * will get its events and not get stuck.
> +	 *
> +	 * Old platforms will also reset the display, so we
> +	 * need to grab the modeset locks around the reset.
> +	 * But in order to do that we must let any pending
> +	 * page flip wait complete since the waiters may be
> +	 * holding some modeset locks.
> +	 */
> +	intel_complete_page_flips(dev);

Is this really required? We complete them afterwards, and all the pageflip
waiters I've found do check for gpu hangs and abort the pageflip wait.
That's already required since the mmio flip might go missing, and thus far
we've only completed the flip _after_ having reset the gpu and gem state
(and grabbed dev->struct_mutex).

> +
> +	/* no reset support for gen2 */
> +	if (IS_GEN2(dev))
> +		return;
> +
> +	/* reset doesn't touch the display */
> +	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
> +		return;
> +
> +	drm_modeset_lock_all(dev);
> +}
> +
> +void intel_finish_reset(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +		return;
> +
> +	/* no reset support for gen2 */
> +	if (IS_GEN2(dev))
> +		return;
> +
> +	/* reset doesn't touch the display */
> +	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
> +		/*
> +		 * Flips in the rings have been nuked by the reset,
> +		 * so update the base address of all primary
> +		 * planes to the the last fb to make sure we're
> +		 * showing the correct fb after a reset.
> +		 */
> +		intel_update_primary_planes(dev);
> +		return;
> +	}
> +
> +	/*
> +	 * The display has been reset as well,
> +	 * so need a full re-initialization.
> +	 */
> +	intel_runtime_pm_disable_interrupts(dev_priv);
> +	intel_runtime_pm_enable_interrupts(dev_priv);
> +
> +	intel_modeset_init_hw(dev);
> +
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	if (dev_priv->display.hpd_irq_setup)
> +		dev_priv->display.hpd_irq_setup(dev);
> +	spin_unlock_irq(&dev_priv->irq_lock);
> +
> +	intel_modeset_setup_hw_state(dev, true);
> +
> +	intel_hpd_init(dev_priv);
> +
> +	drm_modeset_unlock_all(dev);
> +}
> +
>  static int
>  intel_finish_fb(struct drm_framebuffer *old_fb)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f0a46ec..25fdbb1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -958,7 +958,8 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
>  					     unsigned int tiling_mode,
>  					     unsigned int bpp,
>  					     unsigned int pitch);
> -void intel_display_handle_reset(struct drm_device *dev);
> +void intel_prepare_reset(struct drm_device *dev);
> +void intel_finish_reset(struct drm_device *dev);
>  void hsw_enable_pc8(struct drm_i915_private *dev_priv);
>  void hsw_disable_pc8(struct drm_i915_private *dev_priv);
>  void intel_dp_get_m_n(struct intel_crtc *crtc,
> -- 
> 2.0.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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: Grab modeset locks for GPU rest on pre-ctg

[Ville Syrjälä]

On Fri, Nov 21, 2014 at 09:49:21PM +0100, Daniel Vetter wrote:
> On Fri, Nov 21, 2014 at 09:54:29PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > On gen4 and earlier the GPU reset also resets the display, so we should
> > protect against concurrent modeset operations. Grab all the modeset locks
> > around the entire GPU reset dance, remebering first ti dislogde any
> > pending page flip to make sure we don't deadlock. Any pageflip coming
> > in between these two steps should fail anyway due to reset_in_progress,
> > so this should be safe.
> > 
> > This fixes a lot of failed asserts in the modeset code when there's a
> > modeset racing with the reset. Naturally the asserts aren't happy when
> > the expected state has disappeared.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Two comments on this one, otherwise looks good (well didn't bother to
> check the new reset register frobbing).
> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c      | 19 -------
> >  drivers/gpu/drm/i915/i915_irq.c      |  5 +-
> >  drivers/gpu/drm/i915/intel_display.c | 96 ++++++++++++++++++++++++++++++------
> >  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
> >  4 files changed, 86 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 5066fd1..1e9c136 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -880,25 +880,6 @@ int i915_reset(struct drm_device *dev)
> >  		 */
> >  		if (INTEL_INFO(dev)->gen > 5)
> >  			intel_reset_gt_powersave(dev);
> > -
> > -
> > -		if (IS_GEN3(dev) || (IS_GEN4(dev) && !IS_G4X(dev))) {
> > -			intel_runtime_pm_disable_interrupts(dev_priv);
> > -			intel_runtime_pm_enable_interrupts(dev_priv);
> > -
> > -			intel_modeset_init_hw(dev);
> > -
> > -			spin_lock_irq(&dev_priv->irq_lock);
> > -			if (dev_priv->display.hpd_irq_setup)
> > -				dev_priv->display.hpd_irq_setup(dev);
> > -			spin_unlock_irq(&dev_priv->irq_lock);
> > -
> > -			drm_modeset_lock_all(dev);
> > -			intel_modeset_setup_hw_state(dev, true);
> > -			drm_modeset_unlock_all(dev);
> > -
> > -			intel_hpd_init(dev_priv);
> > -		}
> >  	} else {
> >  		mutex_unlock(&dev->struct_mutex);
> >  	}
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 5908580d..8887674 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2428,6 +2428,9 @@ static void i915_error_work_func(struct work_struct *work)
> >  		 * simulated reset via debugs, so get an RPM reference.
> >  		 */
> >  		intel_runtime_pm_get(dev_priv);
> > +
> > +		intel_prepare_reset(dev);
> > +
> >  		/*
> >  		 * All state reset _must_ be completed before we update the
> >  		 * reset counter, for otherwise waiters might miss the reset
> > @@ -2436,7 +2439,7 @@ static void i915_error_work_func(struct work_struct *work)
> >  		 */
> >  		ret = i915_reset(dev);
> >  
> > -		intel_display_handle_reset(dev);
> > +		intel_finish_reset(dev);
> >  
> >  		intel_runtime_pm_put(dev_priv);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 3218455..8329f7c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2765,25 +2765,10 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
> >  	return 0;
> >  }
> >  
> > -void intel_display_handle_reset(struct drm_device *dev)
> > +static void intel_complete_page_flips(struct drm_device *dev)
> >  {
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	struct drm_crtc *crtc;
> >  
> > -	/*
> > -	 * Flips in the rings have been nuked by the reset,
> > -	 * so complete all pending flips so that user space
> > -	 * will get its events and not get stuck.
> > -	 *
> > -	 * Also update the base address of all primary
> > -	 * planes to the the last fb to make sure we're
> > -	 * showing the correct fb after a reset.
> > -	 *
> > -	 * Need to make two loops over the crtcs so that we
> > -	 * don't try to grab a crtc mutex before the
> > -	 * pending_flip_queue really got woken up.
> > -	 */
> > -
> >  	for_each_crtc(dev, crtc) {
> >  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >  		enum plane plane = intel_crtc->plane;
> > @@ -2791,6 +2776,12 @@ void intel_display_handle_reset(struct drm_device *dev)
> >  		intel_prepare_page_flip(dev, plane);
> >  		intel_finish_page_flip_plane(dev, plane);
> >  	}
> > +}
> > +
> > +static void intel_update_primary_planes(struct drm_device *dev)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_crtc *crtc;
> >  
> >  	for_each_crtc(dev, crtc) {
> >  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > @@ -2810,6 +2801,79 @@ void intel_display_handle_reset(struct drm_device *dev)
> >  	}
> >  }
> >  
> > +void intel_prepare_reset(struct drm_device *dev)
> > +{
> > +	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> > +		return;
> 
> ums just officially died please remove. Same for the one below.

okey dokey

> 
> > +
> > +	/*
> > +	 * Flips in the rings will be nuked by the reset,
> > +	 * so complete all pending flips so that user space
> > +	 * will get its events and not get stuck.
> > +	 *
> > +	 * Old platforms will also reset the display, so we
> > +	 * need to grab the modeset locks around the reset.
> > +	 * But in order to do that we must let any pending
> > +	 * page flip wait complete since the waiters may be
> > +	 * holding some modeset locks.
> > +	 */
> > +	intel_complete_page_flips(dev);
> 
> Is this really required? We complete them afterwards, and all the pageflip
> waiters I've found do check for gpu hangs and abort the pageflip wait.
> That's already required since the mmio flip might go missing, and thus far
> we've only completed the flip _after_ having reset the gpu and gem state
> (and grabbed dev->struct_mutex).

Hmm. Yeah, just waking them up ought to be sufficient to dislodge
things. And we already do that before scheduling the error work, but
after setting the reset_in_progress flag, which is very much critical
here. So I guess I could just move the complete pending flips bit to
intel_finish_reset().

But then I do wonder a bit why I originally needed to add the unlocked
page flip complete before the locked .update_plane() call. Did we miss
a wakeup somewhere or did we not abort pending flip waits on reset?

> 
> > +
> > +	/* no reset support for gen2 */
> > +	if (IS_GEN2(dev))
> > +		return;
> > +
> > +	/* reset doesn't touch the display */
> > +	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
> > +		return;
> > +
> > +	drm_modeset_lock_all(dev);
> > +}
> > +
> > +void intel_finish_reset(struct drm_device *dev)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(dev);
> > +
> > +	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> > +		return;
> > +
> > +	/* no reset support for gen2 */
> > +	if (IS_GEN2(dev))
> > +		return;
> > +
> > +	/* reset doesn't touch the display */
> > +	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
> > +		/*
> > +		 * Flips in the rings have been nuked by the reset,
> > +		 * so update the base address of all primary
> > +		 * planes to the the last fb to make sure we're
> > +		 * showing the correct fb after a reset.
> > +		 */
> > +		intel_update_primary_planes(dev);
> > +		return;
> > +	}
> > +
> > +	/*
> > +	 * The display has been reset as well,
> > +	 * so need a full re-initialization.
> > +	 */
> > +	intel_runtime_pm_disable_interrupts(dev_priv);
> > +	intel_runtime_pm_enable_interrupts(dev_priv);
> > +
> > +	intel_modeset_init_hw(dev);
> > +
> > +	spin_lock_irq(&dev_priv->irq_lock);
> > +	if (dev_priv->display.hpd_irq_setup)
> > +		dev_priv->display.hpd_irq_setup(dev);
> > +	spin_unlock_irq(&dev_priv->irq_lock);
> > +
> > +	intel_modeset_setup_hw_state(dev, true);
> > +
> > +	intel_hpd_init(dev_priv);
> > +
> > +	drm_modeset_unlock_all(dev);
> > +}
> > +
> >  static int
> >  intel_finish_fb(struct drm_framebuffer *old_fb)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index f0a46ec..25fdbb1 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -958,7 +958,8 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
> >  					     unsigned int tiling_mode,
> >  					     unsigned int bpp,
> >  					     unsigned int pitch);
> > -void intel_display_handle_reset(struct drm_device *dev);
> > +void intel_prepare_reset(struct drm_device *dev);
> > +void intel_finish_reset(struct drm_device *dev);
> >  void hsw_enable_pc8(struct drm_i915_private *dev_priv);
> >  void hsw_disable_pc8(struct drm_i915_private *dev_priv);
> >  void intel_dp_get_m_n(struct intel_crtc *crtc,
> > -- 
> > 2.0.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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: Grab modeset locks for GPU rest on pre-ctg

[Daniel Vetter]

On Fri, Nov 21, 2014 at 11:10:31PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 21, 2014 at 09:49:21PM +0100, Daniel Vetter wrote:
> > On Fri, Nov 21, 2014 at 09:54:29PM +0200, ville.syrjala@linux.intel.com wrote:
> > > +
> > > +	/*
> > > +	 * Flips in the rings will be nuked by the reset,
> > > +	 * so complete all pending flips so that user space
> > > +	 * will get its events and not get stuck.
> > > +	 *
> > > +	 * Old platforms will also reset the display, so we
> > > +	 * need to grab the modeset locks around the reset.
> > > +	 * But in order to do that we must let any pending
> > > +	 * page flip wait complete since the waiters may be
> > > +	 * holding some modeset locks.
> > > +	 */
> > > +	intel_complete_page_flips(dev);
> > 
> > Is this really required? We complete them afterwards, and all the pageflip
> > waiters I've found do check for gpu hangs and abort the pageflip wait.
> > That's already required since the mmio flip might go missing, and thus far
> > we've only completed the flip _after_ having reset the gpu and gem state
> > (and grabbed dev->struct_mutex).
> 
> Hmm. Yeah, just waking them up ought to be sufficient to dislodge
> things. And we already do that before scheduling the error work, but
> after setting the reset_in_progress flag, which is very much critical
> here. So I guess I could just move the complete pending flips bit to
> intel_finish_reset().
> 
> But then I do wonder a bit why I originally needed to add the unlocked
> page flip complete before the locked .update_plane() call. Did we miss
> a wakeup somewhere or did we not abort pending flip waits on reset?

gpu hang vs. pending flip deadlocks should have been fixed with

commit 17e1df07df0fbc77696a1e1b6ccf9f2e5af70e40
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Sun Sep 8 21:57:13 2013 +0200

    drm/i915: fix wait_for_pending_flips vs gpu hang deadlock

Maybe it's been broken meanwhile but it should have worked since quite a
while. Or are these gen3/4 reset patches really this old?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: Grab modeset locks for GPU rest on pre-ctg

[Ville Syrjälä]

On Mon, Nov 24, 2014 at 10:34:42AM +0100, Daniel Vetter wrote:
> On Fri, Nov 21, 2014 at 11:10:31PM +0200, Ville Syrjälä wrote:
> > On Fri, Nov 21, 2014 at 09:49:21PM +0100, Daniel Vetter wrote:
> > > On Fri, Nov 21, 2014 at 09:54:29PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > +
> > > > +	/*
> > > > +	 * Flips in the rings will be nuked by the reset,
> > > > +	 * so complete all pending flips so that user space
> > > > +	 * will get its events and not get stuck.
> > > > +	 *
> > > > +	 * Old platforms will also reset the display, so we
> > > > +	 * need to grab the modeset locks around the reset.
> > > > +	 * But in order to do that we must let any pending
> > > > +	 * page flip wait complete since the waiters may be
> > > > +	 * holding some modeset locks.
> > > > +	 */
> > > > +	intel_complete_page_flips(dev);
> > > 
> > > Is this really required? We complete them afterwards, and all the pageflip
> > > waiters I've found do check for gpu hangs and abort the pageflip wait.
> > > That's already required since the mmio flip might go missing, and thus far
> > > we've only completed the flip _after_ having reset the gpu and gem state
> > > (and grabbed dev->struct_mutex).
> > 
> > Hmm. Yeah, just waking them up ought to be sufficient to dislodge
> > things. And we already do that before scheduling the error work, but
> > after setting the reset_in_progress flag, which is very much critical
> > here. So I guess I could just move the complete pending flips bit to
> > intel_finish_reset().
> > 
> > But then I do wonder a bit why I originally needed to add the unlocked
> > page flip complete before the locked .update_plane() call. Did we miss
> > a wakeup somewhere or did we not abort pending flip waits on reset?
> 
> gpu hang vs. pending flip deadlocks should have been fixed with
> 
> commit 17e1df07df0fbc77696a1e1b6ccf9f2e5af70e40
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Sun Sep 8 21:57:13 2013 +0200
> 
>     drm/i915: fix wait_for_pending_flips vs gpu hang deadlock
> 
> Maybe it's been broken meanwhile but it should have worked since quite a
> while. Or are these gen3/4 reset patches really this old?

Nah. But I was actually wondering why the unlocked flip complete prior to
.update_plane() was supposedly necessary at all. But yeah the missing
wake_up before that commit explains it.

I'll see about respinning the patch with the flip complete moved back
after the reset. I guess I could actually move it occur even after
.update_plane() now.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 5/6] drm/i915: Grab modeset locks for GPU rest on pre-ctg

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On gen4 and earlier the GPU reset also resets the display, so we should
protect against concurrent modeset operations. Grab all the modeset locks
around the entire GPU reset dance, remebering first ti dislogde any
pending page flip to make sure we don't deadlock. Any pageflip coming
in between these two steps should fail anyway due to reset_in_progress,
so this should be safe.

This fixes a lot of failed asserts in the modeset code when there's a
modeset racing with the reset. Naturally the asserts aren't happy when
the expected state has disappeared.

v2: Drop UMS checks, complete pending flips after the reset (Daniel)

Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      | 19 --------
 drivers/gpu/drm/i915/i915_irq.c      |  5 ++-
 drivers/gpu/drm/i915/intel_display.c | 84 +++++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +-
 4 files changed, 74 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5066fd1..1e9c136 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -880,25 +880,6 @@ int i915_reset(struct drm_device *dev)
 		 */
 		if (INTEL_INFO(dev)->gen > 5)
 			intel_reset_gt_powersave(dev);
-
-
-		if (IS_GEN3(dev) || (IS_GEN4(dev) && !IS_G4X(dev))) {
-			intel_runtime_pm_disable_interrupts(dev_priv);
-			intel_runtime_pm_enable_interrupts(dev_priv);
-
-			intel_modeset_init_hw(dev);
-
-			spin_lock_irq(&dev_priv->irq_lock);
-			if (dev_priv->display.hpd_irq_setup)
-				dev_priv->display.hpd_irq_setup(dev);
-			spin_unlock_irq(&dev_priv->irq_lock);
-
-			drm_modeset_lock_all(dev);
-			intel_modeset_setup_hw_state(dev, true);
-			drm_modeset_unlock_all(dev);
-
-			intel_hpd_init(dev_priv);
-		}
 	} else {
 		mutex_unlock(&dev->struct_mutex);
 	}
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5908580d..8887674 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2428,6 +2428,9 @@ static void i915_error_work_func(struct work_struct *work)
 		 * simulated reset via debugs, so get an RPM reference.
 		 */
 		intel_runtime_pm_get(dev_priv);
+
+		intel_prepare_reset(dev);
+
 		/*
 		 * All state reset _must_ be completed before we update the
 		 * reset counter, for otherwise waiters might miss the reset
@@ -2436,7 +2439,7 @@ static void i915_error_work_func(struct work_struct *work)
 		 */
 		ret = i915_reset(dev);
 
-		intel_display_handle_reset(dev);
+		intel_finish_reset(dev);
 
 		intel_runtime_pm_put(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3218455..56664e6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2765,25 +2765,10 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	return 0;
 }
 
-void intel_display_handle_reset(struct drm_device *dev)
+static void intel_complete_page_flips(struct drm_device *dev)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
 
-	/*
-	 * Flips in the rings have been nuked by the reset,
-	 * so complete all pending flips so that user space
-	 * will get its events and not get stuck.
-	 *
-	 * Also update the base address of all primary
-	 * planes to the the last fb to make sure we're
-	 * showing the correct fb after a reset.
-	 *
-	 * Need to make two loops over the crtcs so that we
-	 * don't try to grab a crtc mutex before the
-	 * pending_flip_queue really got woken up.
-	 */
-
 	for_each_crtc(dev, crtc) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		enum plane plane = intel_crtc->plane;
@@ -2791,6 +2776,12 @@ void intel_display_handle_reset(struct drm_device *dev)
 		intel_prepare_page_flip(dev, plane);
 		intel_finish_page_flip_plane(dev, plane);
 	}
+}
+
+static void intel_update_primary_planes(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
 
 	for_each_crtc(dev, crtc) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -2810,6 +2801,67 @@ void intel_display_handle_reset(struct drm_device *dev)
 	}
 }
 
+void intel_prepare_reset(struct drm_device *dev)
+{
+	/* no reset support for gen2 */
+	if (IS_GEN2(dev))
+		return;
+
+	/* reset doesn't touch the display */
+	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
+		return;
+
+	drm_modeset_lock_all(dev);
+}
+
+void intel_finish_reset(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	/*
+	 * Flips in the rings will be nuked by the reset,
+	 * so complete all pending flips so that user space
+	 * will get its events and not get stuck.
+	 */
+	intel_complete_page_flips(dev);
+
+	/* no reset support for gen2 */
+	if (IS_GEN2(dev))
+		return;
+
+	/* reset doesn't touch the display */
+	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev)) {
+		/*
+		 * Flips in the rings have been nuked by the reset,
+		 * so update the base address of all primary
+		 * planes to the the last fb to make sure we're
+		 * showing the correct fb after a reset.
+		 */
+		intel_update_primary_planes(dev);
+		return;
+	}
+
+	/*
+	 * The display has been reset as well,
+	 * so need a full re-initialization.
+	 */
+	intel_runtime_pm_disable_interrupts(dev_priv);
+	intel_runtime_pm_enable_interrupts(dev_priv);
+
+	intel_modeset_init_hw(dev);
+
+	spin_lock_irq(&dev_priv->irq_lock);
+	if (dev_priv->display.hpd_irq_setup)
+		dev_priv->display.hpd_irq_setup(dev);
+	spin_unlock_irq(&dev_priv->irq_lock);
+
+	intel_modeset_setup_hw_state(dev, true);
+
+	intel_hpd_init(dev_priv);
+
+	drm_modeset_unlock_all(dev);
+}
+
 static int
 intel_finish_fb(struct drm_framebuffer *old_fb)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f0a46ec..25fdbb1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -958,7 +958,8 @@ unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 					     unsigned int tiling_mode,
 					     unsigned int bpp,
 					     unsigned int pitch);
-void intel_display_handle_reset(struct drm_device *dev);
+void intel_prepare_reset(struct drm_device *dev);
+void intel_finish_reset(struct drm_device *dev);
 void hsw_enable_pc8(struct drm_i915_private *dev_priv);
 void hsw_disable_pc8(struct drm_i915_private *dev_priv);
 void intel_dp_get_m_n(struct intel_crtc *crtc,
-- 
2.0.4

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

[PATCH 6/6] drm/i915: Disable crtcs gracefully before GPU reset on gen3/4

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The GPU reset also resets the display on gen3/4. The g33 docs say we
should disable all planes before flipping the reset switch. Just
disable all the crtcs instead. That seems a nicer thing to do anyway.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_uncore.c  |  2 --
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8329f7c..b0980b1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2803,6 +2803,9 @@ static void intel_update_primary_planes(struct drm_device *dev)
 
 void intel_prepare_reset(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_crtc *crtc;
+
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return;
 
@@ -2828,6 +2831,15 @@ void intel_prepare_reset(struct drm_device *dev)
 		return;
 
 	drm_modeset_lock_all(dev);
+
+	/*
+	 * Disabling the crtcs gracefully seems nicer. Also the
+	 * g33 docs say we should at least disable all the planes.
+	 */
+	for_each_intel_crtc(dev, crtc) {
+		if (crtc->active)
+			dev_priv->display.crtc_disable(&crtc->base);
+	}
 }
 
 void intel_finish_reset(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 0f69369..3096c28 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1371,8 +1371,6 @@ static int g4x_reset_complete(struct drm_device *dev)
 
 static int g33_do_reset(struct drm_device *dev)
 {
-	/* FIXME spec says to turn off all planes and wait 1 usec before reset */
-
 	pci_write_config_byte(dev->pdev, I915_GDRST, GRDOM_RESET_ENABLE);
 	return wait_for(g4x_reset_complete(dev), 500);
 }
-- 
2.0.4

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

Re: [PATCH 6/6] drm/i915: Disable crtcs gracefully before

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  367/367              367/367
ILK                 -2              375/375              373/375
SNB                                  450/450              450/450
IVB                 -2              503/503              501/503
BYT                                  289/289              289/289
HSW                 -3              567/567              564/567
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
ILK  igt_gem_reset_stats_close-pending-fork-render      TIMEOUT(3, M37M26)PASS(1, M26)      TIMEOUT(1, M37)
ILK  igt_kms_flip_vblank-vs-hang      TIMEOUT(3, M37M26)PASS(1, M26)      TIMEOUT(1, M37)
IVB  igt_gem_bad_reloc_negative-reloc      NSPT(3, M34M21M4)PASS(1, M21)      NSPT(1, M4)
IVB  igt_gem_bad_reloc_negative-reloc-lut      NSPT(3, M21M34M4)PASS(4, M21M34M4)      NSPT(1, M4)
HSW  igt_gem_bad_reloc_negative-reloc-lut      NSPT(13, M40M20)PASS(1, M20)      NSPT(1, M20)
HSW  igt_kms_rotation_crc_primary-rotation      PASS(14, M20M40)      DMESG_WARN(1, M20)
HSW  igt_pm_rc6_residency_rc6-accuracy      PASS(14, M20M40)      FAIL(1, M20)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/6] drm/i915: Deal with video overlay on GPU reset

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clear the video overlay state on GPU reset. Any pending overlay request
in the ring has been nuked, and the display itself gets reset. So we
pretty much lose all state here. Adjust the software state to match so
that the next "putimage" will restore things to working order.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |  2 ++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_overlay.c | 16 ++++++++++++++++
 3 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1e9c136..71be3c9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -838,6 +838,8 @@ int i915_reset(struct drm_device *dev)
 		return ret;
 	}
 
+	intel_overlay_reset(dev_priv);
+
 	/* Ok, now get things going again... */
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 25fdbb1..1c38c88 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1083,6 +1083,7 @@ int intel_overlay_put_image(struct drm_device *dev, void *data,
 			    struct drm_file *file_priv);
 int intel_overlay_attrs(struct drm_device *dev, void *data,
 			struct drm_file *file_priv);
+void intel_overlay_reset(struct drm_i915_private *dev_priv);
 
 
 /* intel_panel.c */
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index dc2f4f2..24501eb 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -422,6 +422,22 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
 	return 0;
 }
 
+void intel_overlay_reset(struct drm_i915_private *dev_priv)
+{
+	struct intel_overlay *overlay = dev_priv->overlay;
+
+	if (!overlay)
+		return;
+
+	intel_overlay_release_old_vid(overlay);
+
+	overlay->last_flip_req = 0;
+	overlay->old_xscale = 0;
+	overlay->old_yscale = 0;
+	overlay->crtc = NULL;
+	overlay->active = false;
+}
+
 struct put_image_params {
 	int format;
 	short dst_x;
-- 
2.0.4

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

Re: [PATCH 7/6] drm/i915: Deal with video overlay on GPU reset

[Daniel Vetter]

On Mon, Nov 24, 2014 at 06:28:49PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Clear the video overlay state on GPU reset. Any pending overlay request
> in the ring has been nuked, and the display itself gets reset. So we
> pretty much lose all state here. Adjust the software state to match so
> that the next "putimage" will restore things to working order.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      |  2 ++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_overlay.c | 16 ++++++++++++++++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1e9c136..71be3c9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -838,6 +838,8 @@ int i915_reset(struct drm_device *dev)
>  		return ret;
>  	}
>  
> +	intel_overlay_reset(dev_priv);
> +
>  	/* Ok, now get things going again... */
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 25fdbb1..1c38c88 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1083,6 +1083,7 @@ int intel_overlay_put_image(struct drm_device *dev, void *data,
>  			    struct drm_file *file_priv);
>  int intel_overlay_attrs(struct drm_device *dev, void *data,
>  			struct drm_file *file_priv);
> +void intel_overlay_reset(struct drm_i915_private *dev_priv);
>  
>  
>  /* intel_panel.c */
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index dc2f4f2..24501eb 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -422,6 +422,22 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
>  	return 0;
>  }
>  
> +void intel_overlay_reset(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_overlay *overlay = dev_priv->overlay;
> +
> +	if (!overlay)
> +		return;

Some locking checks here would be nice. Since the overlay code currently
holds both dev->struct_mutex and all the modeset locks just a check for
dev->struct_mutex should be good enough. Maybe even add that to
intel_overlay_release_old_vid.
-Daniel

> +
> +	intel_overlay_release_old_vid(overlay);
> +
> +	overlay->last_flip_req = 0;
> +	overlay->old_xscale = 0;
> +	overlay->old_yscale = 0;
> +	overlay->crtc = NULL;
> +	overlay->active = false;
> +}
> +
>  struct put_image_params {
>  	int format;
>  	short dst_x;
> -- 
> 2.0.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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 7/6] drm/i915: Deal with video overlay on GPU reset

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Clear the video overlay state on GPU reset. Any pending overlay request
in the ring has been nuked, and the display itself gets reset. So we
pretty much lose all state here. Adjust the software state to match so
that the next "putimage" will restore things to working order.

v2: Ass a locking check into intel_overlay_release_old_vid() (Daniel)

Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |  2 ++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_overlay.c | 18 ++++++++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1e9c136..71be3c9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -838,6 +838,8 @@ int i915_reset(struct drm_device *dev)
 		return ret;
 	}
 
+	intel_overlay_reset(dev_priv);
+
 	/* Ok, now get things going again... */
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 25fdbb1..1c38c88 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1083,6 +1083,7 @@ int intel_overlay_put_image(struct drm_device *dev, void *data,
 			    struct drm_file *file_priv);
 int intel_overlay_attrs(struct drm_device *dev, void *data,
 			struct drm_file *file_priv);
+void intel_overlay_reset(struct drm_i915_private *dev_priv);
 
 
 /* intel_panel.c */
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index dc2f4f2..2a72892 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -392,6 +392,8 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
 	struct intel_engine_cs *ring = &dev_priv->ring[RCS];
 	int ret;
 
+	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+
 	/* Only wait if there is actually an old frame to release to
 	 * guarantee forward progress.
 	 */
@@ -422,6 +424,22 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
 	return 0;
 }
 
+void intel_overlay_reset(struct drm_i915_private *dev_priv)
+{
+	struct intel_overlay *overlay = dev_priv->overlay;
+
+	if (!overlay)
+		return;
+
+	intel_overlay_release_old_vid(overlay);
+
+	overlay->last_flip_req = 0;
+	overlay->old_xscale = 0;
+	overlay->old_yscale = 0;
+	overlay->crtc = NULL;
+	overlay->active = false;
+}
+
 struct put_image_params {
 	int format;
 	short dst_x;
-- 
2.0.4

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

Re: [PATCH v2 7/6] drm/i915: Deal with video overlay on GPU reset

[Daniel Vetter]

On Wed, Nov 26, 2014 at 05:07:29PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Clear the video overlay state on GPU reset. Any pending overlay request
> in the ring has been nuked, and the display itself gets reset. So we
> pretty much lose all state here. Adjust the software state to match so
> that the next "putimage" will restore things to working order.
> 
> v2: Ass a locking check into intel_overlay_release_old_vid() (Daniel)
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.c      |  2 ++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_overlay.c | 18 ++++++++++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1e9c136..71be3c9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -838,6 +838,8 @@ int i915_reset(struct drm_device *dev)
>  		return ret;
>  	}
>  
> +	intel_overlay_reset(dev_priv);
> +
>  	/* Ok, now get things going again... */
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 25fdbb1..1c38c88 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1083,6 +1083,7 @@ int intel_overlay_put_image(struct drm_device *dev, void *data,
>  			    struct drm_file *file_priv);
>  int intel_overlay_attrs(struct drm_device *dev, void *data,
>  			struct drm_file *file_priv);
> +void intel_overlay_reset(struct drm_i915_private *dev_priv);
>  
>  
>  /* intel_panel.c */
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index dc2f4f2..2a72892 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -392,6 +392,8 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
>  	struct intel_engine_cs *ring = &dev_priv->ring[RCS];
>  	int ret;
>  
> +	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> +
>  	/* Only wait if there is actually an old frame to release to
>  	 * guarantee forward progress.
>  	 */
> @@ -422,6 +424,22 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay)
>  	return 0;
>  }
>  
> +void intel_overlay_reset(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_overlay *overlay = dev_priv->overlay;
> +
> +	if (!overlay)
> +		return;
> +
> +	intel_overlay_release_old_vid(overlay);
> +
> +	overlay->last_flip_req = 0;
> +	overlay->old_xscale = 0;
> +	overlay->old_yscale = 0;
> +	overlay->crtc = NULL;
> +	overlay->active = false;
> +}
> +
>  struct put_image_params {
>  	int format;
>  	short dst_x;
> -- 
> 2.0.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 7/6] drm/i915: Deal with video overlay on GPU reset

[Dave Gordon]

On 26/11/14 18:10, Daniel Vetter wrote:
> On Wed, Nov 26, 2014 at 05:07:29PM +0200, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Clear the video overlay state on GPU reset. Any pending overlay request
>> in the ring has been nuked, and the display itself gets reset. So we
>> pretty much lose all state here. Adjust the software state to match so
>> that the next "putimage" will restore things to working order.
>>
>> v2: Ass a locking check into intel_overlay_release_old_vid() (Daniel)

s/Ass/Add/

.Dave.

>>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Queued for -next, thanks for the patch.
> -Daniel

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