From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: expose energy counter on SNB and IVB
Date: Sun, 24 Jun 2012 12:01:24 +0200 [thread overview]
Message-ID: <20120624100124.GA5087@phenom.ffwll.local> (raw)
In-Reply-To: <20120622190429.63becd32@bwidawsk.net>
On Fri, Jun 22, 2012 at 07:04:29PM -0700, Ben Widawsky wrote:
> On Wed, 20 Jun 2012 14:48:58 -0700
> Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > +static struct attribute *gt_attrs[] = {
> > + &dev_attr_gt_energy_uJ.attr,
> > + NULL,
> > +};
>
> I think convention dictates it should be all lowercase. And while on
> that, gt_energy_uJ is about as descriptive a name as rc6 (what jerk
> named that anyway?). I think something like consumed_microjoules is
> better.
I admit that the uJ makes tons of sense for me - J is the official SI
abbrev. for joules (I'm a bit unsure about u for \mu, but it seems to be
customary). Adding consumed makes some sense I think, but otherwise it's
imo good if we stick with the names vpg ppl have come up. So
gt_consumed_energy_uJ anyone? I can bikeshed this name while applying ...
-Daniel
>
> > +
> > +static struct attribute_group gt_attr_group = {
> > + .name = power_group_name,
> > + .attrs = gt_attrs,
> > +};
> > +
> > static int l3_access_valid(struct drm_device *dev, loff_t offset)
> > {
> > if (!IS_IVYBRIDGE(dev))
> > @@ -217,10 +248,18 @@ void i915_setup_sysfs(struct drm_device *dev)
> > if (ret)
> > DRM_ERROR("l3 parity sysfs setup failed\n");
> > }
> > +
> > + if (IS_GEN6(dev) || IS_IVYBRIDGE(dev)) {
> > + ret = sysfs_merge_group(&dev->primary->kdev.kobj,
> > + >_attr_group);
> > + if (ret)
> > + DRM_ERROR("GT energy sysfs setup failed\n");
> > + }
> > }
> >
> > void i915_teardown_sysfs(struct drm_device *dev)
> > {
> > device_remove_bin_file(&dev->primary->kdev, &dpf_attrs);
> > sysfs_unmerge_group(&dev->primary->kdev.kobj, &rc6_attr_group);
> > + sysfs_unmerge_group(&dev->primary->kdev.kobj, >_attr_group);
> > }
>
> teardown should occur in reverse of init, so I'd put this on top.
>
>
>
> --
> Ben Widawsky, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
next prev parent reply other threads:[~2012-06-24 9:59 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-20 21:48 [PATCH] drm/i915: expose energy counter on SNB and IVB Jesse Barnes
2012-06-23 2:04 ` Ben Widawsky
2012-06-24 10:01 ` Daniel Vetter [this message]
2012-06-24 23:13 ` Eugeni Dodonov
2012-06-24 23:06 ` Eugeni Dodonov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120624100124.GA5087@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=ben@bwidawsk.net \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.