Re: [Intel-gfx] [PATCH] drm/i915/ivb+: Use the correct render forcewake ACK register

2018-02-07 Thread Chris Wilson 
Quoting Imre Deak (2018-02-07 12:48:33)
> On Tue, Feb 06, 2018 at 08:42:23PM +, Chris Wilson wrote:
> > Quoting Imre Deak (2018-02-06 18:20:07)
> > > FORCEWAKE_ACK is depricated by BSpec at least starting from BDW,
> > > referring to the multi-threaded version of it instead. Accessing
> > > FORCEWAKE_ACK triggers an unclaimed register access error - at
> > > least on GLK - see the Reference: below.
> > > 
> > > As opposed to this debugfs file we normally use the MT version on IVB
> > > (if supported) and HSW/BDW, whereas we use FORCEWAKE_ACK_RENDER_GEN9
> > > starting from SKL. Fix the problem by using these same registers on
> > > each platform in the debugfs file too.
> > > 
> > > Cc: Ville Syrjälä 
> > > Cc: Chris Wilson 
> > > Cc: Mika Kuoppala 
> > > Reference: https://bugs.freedesktop.org/show_bug.cgi?id=103337
> > > Signed-off-by: Imre Deak 
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c | 7 +--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 3849ded354e3..8a019cb0980d 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -1458,18 +1458,21 @@ static int vlv_drpc_info(struct seq_file *m)
> > >  static int gen6_drpc_info(struct seq_file *m)
> > >  {
> > > struct drm_i915_private *dev_priv = node_to_i915(m->private);
> > > +   struct intel_uncore_forcewake_domain *render_domain =
> > > +   _priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER];
> > > u32 gt_core_status, rcctl1, rc6vids = 0;
> > > u32 gen9_powergate_enable = 0, gen9_powergate_status = 0;
> > > unsigned forcewake_count;
> > > int count = 0;
> > >  
> > > -   forcewake_count = 
> > > READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
> > > +   forcewake_count = READ_ONCE(render_domain->wake_count);
> > > if (forcewake_count) {
> > > seq_puts(m, "RC information inaccurate because somebody "
> > > "holds a forcewake reference \n");
> > > } else {
> > > /* NB: we cannot use forcewake, else we read the wrong 
> > > values */
> > > -   while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK) 
> > > & 1))
> > > +   while (count++ < 50 &&
> > > +  (I915_READ_NOTRACE(render_domain->reg_ack) & 1))
> > > udelay(10);
> > > seq_printf(m, "RC information accurate: %s\n", 
> > > yesno(count < 51));
> > 
> > I remember having a rant about this being absolutely meaningless. We can
> > just delete it imo.
> 
> Ok, can remove it. IIUC the rational is that the forcewake count is
> provided anyway in the same file and we don't win much by noticing
> a pending forcewake release (without a FW reference), since it's a
> transient state.

It's also "multi-threaded"; what the kernel thinks is only a small part
of the picture.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/ivb+: Use the correct render forcewake ACK register

2018-02-07 Thread Imre Deak 
On Tue, Feb 06, 2018 at 08:42:23PM +, Chris Wilson wrote:
> Quoting Imre Deak (2018-02-06 18:20:07)
> > FORCEWAKE_ACK is depricated by BSpec at least starting from BDW,
> > referring to the multi-threaded version of it instead. Accessing
> > FORCEWAKE_ACK triggers an unclaimed register access error - at
> > least on GLK - see the Reference: below.
> > 
> > As opposed to this debugfs file we normally use the MT version on IVB
> > (if supported) and HSW/BDW, whereas we use FORCEWAKE_ACK_RENDER_GEN9
> > starting from SKL. Fix the problem by using these same registers on
> > each platform in the debugfs file too.
> > 
> > Cc: Ville Syrjälä 
> > Cc: Chris Wilson 
> > Cc: Mika Kuoppala 
> > Reference: https://bugs.freedesktop.org/show_bug.cgi?id=103337
> > Signed-off-by: Imre Deak 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 3849ded354e3..8a019cb0980d 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -1458,18 +1458,21 @@ static int vlv_drpc_info(struct seq_file *m)
> >  static int gen6_drpc_info(struct seq_file *m)
> >  {
> > struct drm_i915_private *dev_priv = node_to_i915(m->private);
> > +   struct intel_uncore_forcewake_domain *render_domain =
> > +   _priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER];
> > u32 gt_core_status, rcctl1, rc6vids = 0;
> > u32 gen9_powergate_enable = 0, gen9_powergate_status = 0;
> > unsigned forcewake_count;
> > int count = 0;
> >  
> > -   forcewake_count = 
> > READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
> > +   forcewake_count = READ_ONCE(render_domain->wake_count);
> > if (forcewake_count) {
> > seq_puts(m, "RC information inaccurate because somebody "
> > "holds a forcewake reference \n");
> > } else {
> > /* NB: we cannot use forcewake, else we read the wrong 
> > values */
> > -   while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK) & 
> > 1))
> > +   while (count++ < 50 &&
> > +  (I915_READ_NOTRACE(render_domain->reg_ack) & 1))
> > udelay(10);
> > seq_printf(m, "RC information accurate: %s\n", yesno(count 
> > < 51));
> 
> I remember having a rant about this being absolutely meaningless. We can
> just delete it imo.

Ok, can remove it. IIUC the rational is that the forcewake count is
provided anyway in the same file and we don't win much by noticing
a pending forcewake release (without a FW reference), since it's a
transient state.

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

Re: [Intel-gfx] [PATCH] drm/i915/ivb+: Use the correct render forcewake ACK register

2018-02-06 Thread Chris Wilson 
Quoting Imre Deak (2018-02-06 18:20:07)
> FORCEWAKE_ACK is depricated by BSpec at least starting from BDW,
> referring to the multi-threaded version of it instead. Accessing
> FORCEWAKE_ACK triggers an unclaimed register access error - at
> least on GLK - see the Reference: below.
> 
> As opposed to this debugfs file we normally use the MT version on IVB
> (if supported) and HSW/BDW, whereas we use FORCEWAKE_ACK_RENDER_GEN9
> starting from SKL. Fix the problem by using these same registers on
> each platform in the debugfs file too.
> 
> Cc: Ville Syrjälä 
> Cc: Chris Wilson 
> Cc: Mika Kuoppala 
> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=103337
> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3849ded354e3..8a019cb0980d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1458,18 +1458,21 @@ static int vlv_drpc_info(struct seq_file *m)
>  static int gen6_drpc_info(struct seq_file *m)
>  {
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
> +   struct intel_uncore_forcewake_domain *render_domain =
> +   _priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER];
> u32 gt_core_status, rcctl1, rc6vids = 0;
> u32 gen9_powergate_enable = 0, gen9_powergate_status = 0;
> unsigned forcewake_count;
> int count = 0;
>  
> -   forcewake_count = 
> READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
> +   forcewake_count = READ_ONCE(render_domain->wake_count);
> if (forcewake_count) {
> seq_puts(m, "RC information inaccurate because somebody "
> "holds a forcewake reference \n");
> } else {
> /* NB: we cannot use forcewake, else we read the wrong values 
> */
> -   while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK) & 1))
> +   while (count++ < 50 &&
> +  (I915_READ_NOTRACE(render_domain->reg_ack) & 1))
> udelay(10);
> seq_printf(m, "RC information accurate: %s\n", yesno(count < 
> 51));

I remember having a rant about this being absolutely meaningless. We can
just delete it imo.
-Chris
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/ivb+: Use the correct render forcewake ACK register

2018-02-06 Thread Imre Deak 
FORCEWAKE_ACK is depricated by BSpec at least starting from BDW,
referring to the multi-threaded version of it instead. Accessing
FORCEWAKE_ACK triggers an unclaimed register access error - at
least on GLK - see the Reference: below.

As opposed to this debugfs file we normally use the MT version on IVB
(if supported) and HSW/BDW, whereas we use FORCEWAKE_ACK_RENDER_GEN9
starting from SKL. Fix the problem by using these same registers on
each platform in the debugfs file too.

Cc: Ville Syrjälä 
Cc: Chris Wilson 
Cc: Mika Kuoppala 
Reference: https://bugs.freedesktop.org/show_bug.cgi?id=103337
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3849ded354e3..8a019cb0980d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1458,18 +1458,21 @@ static int vlv_drpc_info(struct seq_file *m)
 static int gen6_drpc_info(struct seq_file *m)
 {
struct drm_i915_private *dev_priv = node_to_i915(m->private);
+   struct intel_uncore_forcewake_domain *render_domain =
+   _priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER];
u32 gt_core_status, rcctl1, rc6vids = 0;
u32 gen9_powergate_enable = 0, gen9_powergate_status = 0;
unsigned forcewake_count;
int count = 0;
 
-   forcewake_count = 
READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
+   forcewake_count = READ_ONCE(render_domain->wake_count);
if (forcewake_count) {
seq_puts(m, "RC information inaccurate because somebody "
"holds a forcewake reference \n");
} else {
/* NB: we cannot use forcewake, else we read the wrong values */
-   while (count++ < 50 && (I915_READ_NOTRACE(FORCEWAKE_ACK) & 1))
+   while (count++ < 50 &&
+  (I915_READ_NOTRACE(render_domain->reg_ack) & 1))
udelay(10);
seq_printf(m, "RC information accurate: %s\n", yesno(count < 
51));
}
-- 
2.13.2

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