Bug 84524 - Please don't export RC6p or RC6pp if they don't exist
Summary: Please don't export RC6p or RC6pp if they don't exist
Status: CLOSED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: Intel GFX Bugs mailing list
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-30 17:36 UTC by Josh Triplett
Modified: 2017-07-24 22:51 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
patch (4.53 KB, patch)
2014-09-30 22:01 UTC, Rodrigo Vivi
no flags Details | Splinter Review

Description Josh Triplett 2014-09-30 17:36:01 UTC
Currently, powertop displays RC6p and RC6pp on all platforms, even though current hardware just has one unified RC6 state.  This generates support questions ("why isn't my system getting into a deeper sleep state?").

The kernel driver should not export rc6p_residency_ms or rc6pp_residency_ms at all on hardware platforms that don't have those distinct states.  We can then teach powertop to only display the states for which _residency_ms files exist.  (I've confirmed that existing versions of powertop will simply display 0 residency if the files don't exist, as though they existed and contained 0.)
Comment 1 Rodrigo Vivi 2014-09-30 22:01:05 UTC
Created attachment 107155 [details] [review]
patch
Comment 2 Rodrigo Vivi 2014-09-30 22:02:23 UTC
The patch is here and sent to mailing list.
However I'm closing the bug anyway since the powertop behaviour described isn't true. I just tested my patch with latest powertop v2.6.1 and it still shows RC6p and RC6pp as 0%.
Comment 3 Josh Triplett 2014-10-01 21:10:35 UTC
(In reply to comment #2)
> The patch is here and sent to mailing list.
> However I'm closing the bug anyway since the powertop behaviour described
> isn't true. I just tested my patch with latest powertop v2.6.1 and it still
> shows RC6p and RC6pp as 0%.

That's exactly the behavior that I said current powertop has; thus, this change will not break existing versions of powertop.  I'm proposing this change because then *new* versions of powertop can stop displaying these fields entirely.
Comment 4 Paulo Zanoni 2014-10-07 20:18:59 UTC
Josh, please notice that even though both Sandybridge and Ivybdrige support RC6p and RC6pp, by default we don't ever enable RC6pp on any platform, and we only enable RC6p on Ivybridge. So we're still going to have the "Why isn't my system getting into a deeper sleep state?" questions for the next decades. The advantage is that only people with SNB  and IVB will be asking this.

Since we're probably still going to have the questions, setting up a webpage with an answer and an IRC bot to parse questions and provide the FAQ URL would be a better bug fix.

Still, let's work to get Rodrigo's patch merged.
Comment 5 Rodrigo Vivi 2014-10-09 20:58:18 UTC
Patch merged upstream. Thanks for the request/suggestion.

commit bb530a44ceaec228bdf7329e303875d2cc82c115
Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
Date:   Tue Oct 7 07:06:50 2014 -0700

    drm/i915: Do not export RC6p and RC6pp if they don't exist


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.