Re: [Intel-gfx] [PATCH] drm/i915/ivb+: Use the correct render forcewake ACK register
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 = > > > + &dev_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
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 = > > + &dev_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
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 = > + &dev_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