On Wed, Nov 23, 2011 at 11:32:31AM -0200, Eugeni Dodonov wrote: > On Tue, Nov 22, 2011 at 20:58, Ben Widawsky <ben at bwidawsk.net> wrote: > > > Many of the old fields from Ironlake have gone away. Strip all those > > fields, and try to update to fields people care about. RC information > > isn't exactly ideal anymore. All we can guarantee when we read the > > register is that we're not using forcewake, ie. the software isn't > > forcing the hardware to stay awake. The downside is that in doing this > > we may wait a while and that causes an unnaturally idle state on the > > GPU. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42578 > > Cc: Eugeni Dodonov <eugeni.dodonov at intel.com> > > Signed-off-by: Ben Widawsky <ben at bwidawsk.net> > > > > For both patches: > Reviewed-by: Eugeni Dodonov <eugeni.dodonov at intel.com> > > Just one comment below. > > > > + if (count == 50) > > + DRM_INFO("Couldn't get accurate RC information\n"); > > > > Maybe we should seq_printf() this as well? Like, for example: > > seq_printf(m, "RC information accurate: %s\n", yesno(count != 50)); > > So the userspace consulting this file would know if it should rely on the > values or retry the read.. > > Just an idea. It's a good idea... I should almost definitely do that since the information is *almost* definitely incorrect (unless we got really lucky and the bit flipped after we gave up). Also on second thought, I should probably check if user forcewake is set, in which case I can recommend they unset it before reading the value. I'll redo this. Thanks. Ben