Re: [PATCH 09/17] drm/i915: Report requested frequency alongside current frequency in debugfs

Chris Wilson
2013-08-27T12:36:29Z

On Tue, Aug 27, 2013 at 09:12:40AM -0300, Rodrigo Vivi wrote:
> On Mon, Aug 26, 2013 at 7:51 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > It can be useful to compare at times the current vs requested frequency
> > of the GPU, so provide the contents of RPNSWREQ alonside CAGF.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 55ab924..a6f4cb5 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -857,7 +857,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
> > u32 gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
> > u32 rp_state_limits = I915_READ(GEN6_RP_STATE_LIMITS);
> > u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> > - u32 rpstat, cagf;
> > + u32 rpstat, cagf, reqf;
> > u32 rpupei, rpcurup, rpprevup;
> > u32 rpdownei, rpcurdown, rpprevdown;
> > int max_freq;
> > @@ -869,6 +869,14 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
> >
> > gen6_gt_force_wake_get(dev_priv);
> >
> > + reqf = I915_READ(GEN6_RPNSWREQ);
> > + reqf &= ~GEN6_TURBO_DISABLE;
> > + if (IS_HASWELL(dev))
> > + reqf >>= 24;
> > + else
> > + reqf >>= 25;
>
> I would prefer a define like HSW_REQF_SHIFT 24 close to HSW_FREQUENCY
> to avoid people asking why magic 24/25 value.
> I added the HSW_FREQUENCY there and still asked "why 24?"

Heck, I wrote it straight out of the bspec, and was too lazy to chase
the meaning of CAGF_SHIFT simply because it is not obvious from reading
the code and require header chasing.

I'll let our hard working maintainer decide how he wants it if he
applies it. For longer term, I have this exported through perf anyway.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre