[PATCH] drm/i915: Disable mmio debugging during user access

[PATCH] drm/i915: Disable mmio debugging during user access

[Chris Wilson]

If the user bypasses i915 and accesses mmio directly, that easily
confuses our automatic mmio debugging (any error we then detect is
likely to be as a result of the user). Since we expect userspace to open
debugfs/i915_forcewake_user if i915.ko is loaded and they want mmio
access, that makes the opportune time to disable our debugging for
duration of the bypass.

References: https://bugs.freedesktop.org/show_bug.cgi?id=102543
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 32 ++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_uncore.h |  7 +++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 48572b157222..d63b3c5c2f83 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4679,7 +4679,21 @@ static int i915_forcewake_open(struct inode *inode, struct file *file)
 		return 0;
 
 	intel_runtime_pm_get(dev_priv);
-	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+
+	spin_lock_irq(&dev_priv->uncore.lock);
+	if (!dev_priv->uncore.user_forcewake.count++) {
+		intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
+
+		/* Save and disable mmio debugging for the user bypass */
+		dev_priv->uncore.user_forcewake.saved_mmio_check =
+			dev_priv->uncore.unclaimed_mmio_check;
+		dev_priv->uncore.user_forcewake.saved_mmio_debug =
+			i915.mmio_debug;
+
+		dev_priv->uncore.unclaimed_mmio_check = 0;
+		i915.mmio_debug = 0;
+	}
+	spin_unlock_irq(&dev_priv->uncore.lock);
 
 	return 0;
 }
@@ -4691,7 +4705,21 @@ static int i915_forcewake_release(struct inode *inode, struct file *file)
 	if (INTEL_GEN(dev_priv) < 6)
 		return 0;
 
-	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+	spin_lock_irq(&dev_priv->uncore.lock);
+	if (!--dev_priv->uncore.user_forcewake.count) {
+		if (intel_uncore_unclaimed_mmio(dev_priv))
+			dev_info(dev_priv->drm.dev,
+				 "Unclaimed mmio occurred during user acess\n");
+
+		dev_priv->uncore.unclaimed_mmio_check =
+			dev_priv->uncore.user_forcewake.saved_mmio_check;
+		i915.mmio_debug =
+			dev_priv->uncore.user_forcewake.saved_mmio_debug;
+
+		intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL);
+	}
+	spin_unlock_irq(&dev_priv->uncore.lock);
+
 	intel_runtime_pm_put(dev_priv);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 5f90278da461..1acf24c6e9ce 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -102,6 +102,13 @@ struct intel_uncore {
 		i915_reg_t reg_ack;
 	} fw_domain[FW_DOMAIN_ID_COUNT];
 
+	struct {
+		unsigned int count;
+
+		int saved_mmio_check;
+		int saved_mmio_debug;
+	} user_forcewake;
+
 	int unclaimed_mmio_check;
 };
 
-- 
2.14.1

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

✓ Fi.CI.BAT: success for drm/i915: Disable mmio debugging during user access

[Patchwork]

== Series Details ==

Series: drm/i915: Disable mmio debugging during user access
URL   : https://patchwork.freedesktop.org/series/29935/
State : success

== Summary ==

Series 29935v1 drm/i915: Disable mmio debugging during user access
https://patchwork.freedesktop.org/api/1.0/series/29935/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                skip       -> PASS       (fi-cfl-s) fdo#102294
Test drv_module_reload:
        Subgroup basic-reload-inject:
                dmesg-fail -> DMESG-WARN (fi-cfl-s) k.org#196765

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
k.org#196765 https://bugzilla.kernel.org/show_bug.cgi?id=196765

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:461s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:439s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:357s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:551s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:255s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:523s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:523s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:513s
fi-cfl-s         total:289  pass:250  dwarn:4   dfail:0   fail:0   skip:35  time:474s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:436s
fi-glk-2a        total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:615s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:445s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:424s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:418s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:507s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:473s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:517s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:599s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:599s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:531s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:472s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:543s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:520s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:443s
fi-skl-x1585l    total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:491s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:551s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:402s

00f9b49384df3d7874273e1368c770cc651464df drm-tip: 2017y-09m-07d-09h-58m-50s UTC integration manifest
8977571d858e drm/i915: Disable mmio debugging during user access

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5605/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Disable mmio debugging during user access

[Mika Kuoppala]

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

> If the user bypasses i915 and accesses mmio directly, that easily
> confuses our automatic mmio debugging (any error we then detect is
> likely to be as a result of the user). Since we expect userspace to open
> debugfs/i915_forcewake_user if i915.ko is loaded and they want mmio
> access, that makes the opportune time to disable our debugging for
> duration of the bypass.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=102543
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 32 ++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_uncore.h |  7 +++++++
>  2 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 48572b157222..d63b3c5c2f83 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4679,7 +4679,21 @@ static int i915_forcewake_open(struct inode *inode, struct file *file)
>  		return 0;
>  
>  	intel_runtime_pm_get(dev_priv);
> -	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> +
> +	spin_lock_irq(&dev_priv->uncore.lock);
> +	if (!dev_priv->uncore.user_forcewake.count++) {
> +		intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
> +
> +		/* Save and disable mmio debugging for the user bypass */
> +		dev_priv->uncore.user_forcewake.saved_mmio_check =
> +			dev_priv->uncore.unclaimed_mmio_check;
> +		dev_priv->uncore.user_forcewake.saved_mmio_debug =
> +			i915.mmio_debug;
> +
> +		dev_priv->uncore.unclaimed_mmio_check = 0;
> +		i915.mmio_debug = 0;
> +	}
> +	spin_unlock_irq(&dev_priv->uncore.lock);

Just pondering if this should be in intel_uncore.c as a
user_mmio_bypass_on/off(). But not insisting.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

>  
>  	return 0;
>  }
> @@ -4691,7 +4705,21 @@ static int i915_forcewake_release(struct inode *inode, struct file *file)
>  	if (INTEL_GEN(dev_priv) < 6)
>  		return 0;
>  
> -	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> +	spin_lock_irq(&dev_priv->uncore.lock);
> +	if (!--dev_priv->uncore.user_forcewake.count) {
> +		if (intel_uncore_unclaimed_mmio(dev_priv))
> +			dev_info(dev_priv->drm.dev,
> +				 "Unclaimed mmio occurred during user acess\n");
> +
> +		dev_priv->uncore.unclaimed_mmio_check =
> +			dev_priv->uncore.user_forcewake.saved_mmio_check;
> +		i915.mmio_debug =
> +			dev_priv->uncore.user_forcewake.saved_mmio_debug;
> +
> +		intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL);
> +	}
> +	spin_unlock_irq(&dev_priv->uncore.lock);
> +
>  	intel_runtime_pm_put(dev_priv);
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index 5f90278da461..1acf24c6e9ce 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -102,6 +102,13 @@ struct intel_uncore {
>  		i915_reg_t reg_ack;
>  	} fw_domain[FW_DOMAIN_ID_COUNT];
>  
> +	struct {
> +		unsigned int count;
> +
> +		int saved_mmio_check;
> +		int saved_mmio_debug;
> +	} user_forcewake;
> +
>  	int unclaimed_mmio_check;
>  };
>  
> -- 
> 2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Disable mmio debugging during user access

[Chris Wilson]

Quoting Mika Kuoppala (2017-09-07 14:22:21)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > If the user bypasses i915 and accesses mmio directly, that easily
> > confuses our automatic mmio debugging (any error we then detect is
> > likely to be as a result of the user). Since we expect userspace to open
> > debugfs/i915_forcewake_user if i915.ko is loaded and they want mmio
> > access, that makes the opportune time to disable our debugging for
> > duration of the bypass.
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=102543
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 32 ++++++++++++++++++++++++++++++--
> >  drivers/gpu/drm/i915/intel_uncore.h |  7 +++++++
> >  2 files changed, 37 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 48572b157222..d63b3c5c2f83 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -4679,7 +4679,21 @@ static int i915_forcewake_open(struct inode *inode, struct file *file)
> >               return 0;
> >  
> >       intel_runtime_pm_get(dev_priv);
> > -     intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> > +
> > +     spin_lock_irq(&dev_priv->uncore.lock);
> > +     if (!dev_priv->uncore.user_forcewake.count++) {
> > +             intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
> > +
> > +             /* Save and disable mmio debugging for the user bypass */
> > +             dev_priv->uncore.user_forcewake.saved_mmio_check =
> > +                     dev_priv->uncore.unclaimed_mmio_check;
> > +             dev_priv->uncore.user_forcewake.saved_mmio_debug =
> > +                     i915.mmio_debug;
> > +
> > +             dev_priv->uncore.unclaimed_mmio_check = 0;
> > +             i915.mmio_debug = 0;
> > +     }
> > +     spin_unlock_irq(&dev_priv->uncore.lock);
> 
> Just pondering if this should be in intel_uncore.c as a
> user_mmio_bypass_on/off(). But not insisting.

Good call. Fwiw, since CI didn't report the improvement, this did shut up
tools_tests:
[   37.601863] i915 0000:00:02.0: Unclaimed mmio occurred during user acess

Ah better fix the spelling as well.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Disable mmio debugging during user access

[Patchwork]

== Series Details ==

Series: drm/i915: Disable mmio debugging during user access
URL   : https://patchwork.freedesktop.org/series/29935/
State : success

== Summary ==

Test kms_atomic_transition:
        Subgroup plane-use-after-nonblocking-unbind:
                incomplete -> FAIL       (shard-hsw) fdo#101847
Test perf:
        Subgroup blocking:
                pass       -> FAIL       (shard-hsw) fdo#102252

fdo#101847 https://bugs.freedesktop.org/show_bug.cgi?id=101847
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2265 pass:1234 dwarn:0   dfail:0   fail:15  skip:1016 time:9651s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5605/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915: Disable mmio debugging during user access

[Chris Wilson]

If the user bypasses i915 and accesses mmio directly, that easily
confuses our automatic mmio debugging (any error we then detect is
likely to be as a result of the user). Since we expect userspace to open
debugfs/i915_forcewake_user if i915.ko is loaded and they want mmio
access, that makes the opportune time to disable our debugging for
duration of the bypass.

v2: Move the fiddling of uncore internals to uncore.c

References: https://bugs.freedesktop.org/show_bug.cgi?id=102543
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++------
 drivers/gpu/drm/i915/intel_uncore.c | 51 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uncore.h | 10 ++++++++
 3 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 48572b157222..87673efc52fe 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1421,6 +1421,9 @@ static int i915_forcewake_domains(struct seq_file *m, void *data)
 	struct intel_uncore_forcewake_domain *fw_domain;
 	unsigned int tmp;
 
+	seq_printf(m, "user.bypass_count = %u\n",
+		   i915->uncore.user_forcewake.count);
+
 	for_each_fw_domain(fw_domain, i915, tmp)
 		seq_printf(m, "%s.wake_count = %u\n",
 			   intel_uncore_forcewake_domain_to_str(fw_domain->id),
@@ -4673,26 +4676,26 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
 
 static int i915_forcewake_open(struct inode *inode, struct file *file)
 {
-	struct drm_i915_private *dev_priv = inode->i_private;
+	struct drm_i915_private *i915 = inode->i_private;
 
-	if (INTEL_GEN(dev_priv) < 6)
+	if (INTEL_GEN(i915) < 6)
 		return 0;
 
-	intel_runtime_pm_get(dev_priv);
-	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+	intel_runtime_pm_get(i915);
+	intel_uncore_forcewake_user_get(i915);
 
 	return 0;
 }
 
 static int i915_forcewake_release(struct inode *inode, struct file *file)
 {
-	struct drm_i915_private *dev_priv = inode->i_private;
+	struct drm_i915_private *i915 = inode->i_private;
 
-	if (INTEL_GEN(dev_priv) < 6)
+	if (INTEL_GEN(i915) < 6)
 		return 0;
 
-	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
-	intel_runtime_pm_put(dev_priv);
+	intel_uncore_forcewake_user_put(i915);
+	intel_runtime_pm_put(i915);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 0529af7cfbb8..1b38eb94d461 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -489,6 +489,57 @@ void intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+/**
+ * intel_uncore_forcewake_user_get - claim forcewake on behalf of userspace
+ * @dev_priv: i915 device instance
+ *
+ * This function is a wrapper around intel_uncore_forcewake_get() to acquire
+ * the GT powerwell and in the process disable our debugging for the
+ * duration of userspace's bypass.
+ */
+void intel_uncore_forcewake_user_get(struct drm_i915_private *dev_priv)
+{
+	spin_lock_irq(&dev_priv->uncore.lock);
+	if (!dev_priv->uncore.user_forcewake.count++) {
+		intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
+
+		/* Save and disable mmio debugging for the user bypass */
+		dev_priv->uncore.user_forcewake.saved_mmio_check =
+			dev_priv->uncore.unclaimed_mmio_check;
+		dev_priv->uncore.user_forcewake.saved_mmio_debug =
+			i915.mmio_debug;
+
+		dev_priv->uncore.unclaimed_mmio_check = 0;
+		i915.mmio_debug = 0;
+	}
+	spin_unlock_irq(&dev_priv->uncore.lock);
+}
+
+/**
+ * intel_uncore_forcewake_user_put - release forcewake on behalf of userspace
+ * @dev_priv: i915 device instance
+ *
+ * This function complements intel_uncore_forcewake_user_get() and releases
+ * the GT powerwell taken on behalf of the userspace bypass.
+ */
+void intel_uncore_forcewake_user_put(struct drm_i915_private *dev_priv)
+{
+	spin_lock_irq(&dev_priv->uncore.lock);
+	if (!--dev_priv->uncore.user_forcewake.count) {
+		if (intel_uncore_unclaimed_mmio(dev_priv))
+			dev_info(dev_priv->drm.dev,
+				 "Invalid mmio detected during user access\n");
+
+		dev_priv->uncore.unclaimed_mmio_check =
+			dev_priv->uncore.user_forcewake.saved_mmio_check;
+		i915.mmio_debug =
+			dev_priv->uncore.user_forcewake.saved_mmio_debug;
+
+		intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL);
+	}
+	spin_unlock_irq(&dev_priv->uncore.lock);
+}
+
 /**
  * intel_uncore_forcewake_get__locked - grab forcewake domain references
  * @dev_priv: i915 device instance
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 5f90278da461..03786f931905 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -102,6 +102,13 @@ struct intel_uncore {
 		i915_reg_t reg_ack;
 	} fw_domain[FW_DOMAIN_ID_COUNT];
 
+	struct {
+		unsigned int count;
+
+		int saved_mmio_check;
+		int saved_mmio_debug;
+	} user_forcewake;
+
 	int unclaimed_mmio_check;
 };
 
@@ -144,6 +151,9 @@ void intel_uncore_forcewake_get__locked(struct drm_i915_private *dev_priv,
 void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
 					enum forcewake_domains domains);
 
+void intel_uncore_forcewake_user_get(struct drm_i915_private *dev_priv);
+void intel_uncore_forcewake_user_put(struct drm_i915_private *dev_priv);
+
 int intel_wait_for_register(struct drm_i915_private *dev_priv,
 			    i915_reg_t reg,
 			    u32 mask,
-- 
2.14.1

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

Re: [PATCH v2] drm/i915: Disable mmio debugging during user access

[Michal Wajdeczko]

On Thu, Sep 07, 2017 at 02:44:41PM +0100, Chris Wilson wrote:
> If the user bypasses i915 and accesses mmio directly, that easily
> confuses our automatic mmio debugging (any error we then detect is
> likely to be as a result of the user). Since we expect userspace to open
> debugfs/i915_forcewake_user if i915.ko is loaded and they want mmio
> access, that makes the opportune time to disable our debugging for
> duration of the bypass.
> 
> v2: Move the fiddling of uncore internals to uncore.c
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=102543
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---

<snip>

> +/**
> + * intel_uncore_forcewake_user_get - claim forcewake on behalf of userspace
> + * @dev_priv: i915 device instance
> + *
> + * This function is a wrapper around intel_uncore_forcewake_get() to acquire
> + * the GT powerwell and in the process disable our debugging for the
> + * duration of userspace's bypass.
> + */
> +void intel_uncore_forcewake_user_get(struct drm_i915_private *dev_priv)

Maybe little off-topic: This file already uses in many places "i915" as a name
for the "drm_i915_private" param (which is the preferred name over "dev_priv").
Why for this new function you are still using legacy name ?

Is it due to the fact that you want to avoid name overload as you want to
access global "i915"? If so, what is our long term plan to avoid such clashes.

Thanks,
Michal

> +{
> +	spin_lock_irq(&dev_priv->uncore.lock);
> +	if (!dev_priv->uncore.user_forcewake.count++) {
> +		intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
> +
> +		/* Save and disable mmio debugging for the user bypass */
> +		dev_priv->uncore.user_forcewake.saved_mmio_check =
> +			dev_priv->uncore.unclaimed_mmio_check;
> +		dev_priv->uncore.user_forcewake.saved_mmio_debug =
> +			i915.mmio_debug;
                        ^^^^

> +
> +		dev_priv->uncore.unclaimed_mmio_check = 0;
> +		i915.mmio_debug = 0;
                ^^^^

> +	}
> +	spin_unlock_irq(&dev_priv->uncore.lock);
> +}
> +
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Disable mmio debugging during user access

[Chris Wilson]

Quoting Michal Wajdeczko (2017-09-07 16:55:39)
> On Thu, Sep 07, 2017 at 02:44:41PM +0100, Chris Wilson wrote:
> > If the user bypasses i915 and accesses mmio directly, that easily
> > confuses our automatic mmio debugging (any error we then detect is
> > likely to be as a result of the user). Since we expect userspace to open
> > debugfs/i915_forcewake_user if i915.ko is loaded and they want mmio
> > access, that makes the opportune time to disable our debugging for
> > duration of the bypass.
> > 
> > v2: Move the fiddling of uncore internals to uncore.c
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=102543
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> 
> <snip>
> 
> > +/**
> > + * intel_uncore_forcewake_user_get - claim forcewake on behalf of userspace
> > + * @dev_priv: i915 device instance
> > + *
> > + * This function is a wrapper around intel_uncore_forcewake_get() to acquire
> > + * the GT powerwell and in the process disable our debugging for the
> > + * duration of userspace's bypass.
> > + */
> > +void intel_uncore_forcewake_user_get(struct drm_i915_private *dev_priv)
> 
> Maybe little off-topic: This file already uses in many places "i915" as a name
> for the "drm_i915_private" param (which is the preferred name over "dev_priv").
> Why for this new function you are still using legacy name ?
> 
> Is it due to the fact that you want to avoid name overload as you want to
> access global "i915"? If so, what is our long term plan to avoid such clashes.

Bingo. i915_params. Plus also killing off modules options as often as
possible. Very few things should exist at the module level and not the
device level or below, and as a rule of thumb all are workarounds for
driver bugs. (The most acceptable are for testing quirks, at some point
you have to ask a user if the hw works with a certain flag set or not,
before we encapsulate that into a quirk table.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Disable mmio debugging during user access (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Disable mmio debugging during user access (rev2)
URL   : https://patchwork.freedesktop.org/series/29935/
State : success

== Summary ==

Series 29935v2 drm/i915: Disable mmio debugging during user access
https://patchwork.freedesktop.org/api/1.0/series/29935/revisions/2/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> FAIL       (fi-skl-6700k) fdo#100367
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                skip       -> PASS       (fi-cfl-s) fdo#102294
Test drv_module_reload:
        Subgroup basic-reload-inject:
                dmesg-fail -> DMESG-WARN (fi-cfl-s) k.org#196765

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
k.org#196765 https://bugzilla.kernel.org/show_bug.cgi?id=196765

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:461s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:445s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:367s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:564s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:256s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:526s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:528s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:531s
fi-cfl-s         total:289  pass:250  dwarn:4   dfail:0   fail:0   skip:35  time:469s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:438s
fi-glk-2a        total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:627s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:453s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:427s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:430s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:501s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:478s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:518s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:601s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:603s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:531s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:477s
fi-skl-6700k     total:289  pass:264  dwarn:0   dfail:0   fail:1   skip:24  time:543s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:521s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:446s
fi-skl-x1585l    total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:504s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:566s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:414s

00f9b49384df3d7874273e1368c770cc651464df drm-tip: 2017y-09m-07d-09h-58m-50s UTC integration manifest
ed674bc9a3ff drm/i915: Disable mmio debugging during user access

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5607/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Disable mmio debugging during user access (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Disable mmio debugging during user access (rev2)
URL   : https://patchwork.freedesktop.org/series/29935/
State : success

== Summary ==

Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test perf:
        Subgroup blocking:
                pass       -> FAIL       (shard-hsw) fdo#102252
Test kms_atomic_transition:
        Subgroup plane-use-after-nonblocking-unbind:
                incomplete -> FAIL       (shard-hsw) fdo#101847

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#101847 https://bugs.freedesktop.org/show_bug.cgi?id=101847

shard-hsw        total:2265 pass:1235 dwarn:0   dfail:0   fail:14  skip:1016 time:9584s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5607/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for drm/i915: Disable mmio debugging during user access (rev2)

[Chris Wilson]

Quoting Patchwork (2017-09-07 17:52:37)
> == Series Details ==
> 
> Series: drm/i915: Disable mmio debugging during user access (rev2)
> URL   : https://patchwork.freedesktop.org/series/29935/
> State : success
> 
> == Summary ==
> 
> Test kms_setmode:
>         Subgroup basic:
>                 fail       -> PASS       (shard-hsw) fdo#99912
> Test perf:
>         Subgroup blocking:
>                 pass       -> FAIL       (shard-hsw) fdo#102252
> Test kms_atomic_transition:
>         Subgroup plane-use-after-nonblocking-unbind:
>                 incomplete -> FAIL       (shard-hsw) fdo#101847

Pushed to silence another test, although figuring out why intel_reg_dump
is reading an invalid register might of interest to someone.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx