[PATCH 1/1] Revert "drm/i915: drop i915_ prefix from enable_rc6, enable_fbc, enable_ppgtt parameters"

[PATCH 1/1] Revert "drm/i915: drop i915_ prefix from enable_rc6, enable_fbc, enable_ppgtt parameters"

[Amit Shah]

This reverts commit 3adee7a7976012a20f1d3b5a529a3c105e29fef1.

After upgrading to v3.15, my laptop's battery started draining quite
fast.  Powertop pointed to the deep RC6 states not being used.  The
kernel param I had put to enable them had stopped working the way it
used to; so I disagree with the 'not maintaing ABI' part of the param
name change.

However weird the names may be, they're in active use and changing them
only causes pain for users.  This also isn't advertised (marked
deprecated, big warning shown, etc.), so just reverting now.

CC: Daniel Vetter <daniel.vetter@ffwll.ch>
CC: Jani Nikula <jani.nikula@linux.intel.com>
CC: David Airlie <airlied@linux.ie>
CC: <stable@vger.kernel.org> # v3.15+
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/gpu/drm/i915/i915_params.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index d05a2af..053981d 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -69,16 +69,16 @@ MODULE_PARM_DESC(semaphores,
 	"Use semaphores for inter-ring sync "
 	"(default: -1 (use per-chip defaults))");
 
-module_param_named(enable_rc6, i915.enable_rc6, int, 0400);
-MODULE_PARM_DESC(enable_rc6,
+module_param_named(i915_enable_rc6, i915.enable_rc6, int, 0400);
+MODULE_PARM_DESC(i915_enable_rc6,
 	"Enable power-saving render C-state 6. "
 	"Different stages can be selected via bitmask values "
 	"(0 = disable; 1 = enable rc6; 2 = enable deep rc6; 4 = enable deepest rc6). "
 	"For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. "
 	"default: -1 (use per-chip default)");
 
-module_param_named(enable_fbc, i915.enable_fbc, int, 0600);
-MODULE_PARM_DESC(enable_fbc,
+module_param_named(i915_enable_fbc, i915.enable_fbc, int, 0600);
+MODULE_PARM_DESC(i915_enable_fbc,
 	"Enable frame buffer compression for power savings "
 	"(default: -1 (use per-chip default))");
 
@@ -111,8 +111,8 @@ MODULE_PARM_DESC(enable_hangcheck,
 	"WARNING: Disabling this can cause system wide hangs. "
 	"(default: true)");
 
-module_param_named(enable_ppgtt, i915.enable_ppgtt, int, 0400);
-MODULE_PARM_DESC(enable_ppgtt,
+module_param_named(i915_enable_ppgtt, i915.enable_ppgtt, int, 0400);
+MODULE_PARM_DESC(i915_enable_ppgtt,
 	"Override PPGTT usage. "
 	"(-1=auto [default], 0=disabled, 1=aliasing, 2=full)");
 
-- 
1.9.3

Re: [PATCH 1/1] Revert "drm/i915: drop i915_ prefix from enable_rc6, enable_fbc, enable_ppgtt parameters"

[Linus Torvalds]

[-- Attachment #1.1: Type: text/plain, Size: 2920 bytes --]

Sorry for the top post, I'm on the road..

In wondering if we couldn't just keep both the old an the new names and
have them both point at the same variable? Remove the description for the
old name, but keep it working?

    Linus
On Jul 16, 2014 8:34 AM, "Amit Shah" <amit.shah@redhat.com> wrote:

> This reverts commit 3adee7a7976012a20f1d3b5a529a3c105e29fef1.
>
> After upgrading to v3.15, my laptop's battery started draining quite
> fast.  Powertop pointed to the deep RC6 states not being used.  The
> kernel param I had put to enable them had stopped working the way it
> used to; so I disagree with the 'not maintaing ABI' part of the param
> name change.
>
> However weird the names may be, they're in active use and changing them
> only causes pain for users.  This also isn't advertised (marked
> deprecated, big warning shown, etc.), so just reverting now.
>
> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> CC: Jani Nikula <jani.nikula@linux.intel.com>
> CC: David Airlie <airlied@linux.ie>
> CC: <stable@vger.kernel.org> # v3.15+
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  drivers/gpu/drm/i915/i915_params.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index d05a2af..053981d 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -69,16 +69,16 @@ MODULE_PARM_DESC(semaphores,
>         "Use semaphores for inter-ring sync "
>         "(default: -1 (use per-chip defaults))");
>
> -module_param_named(enable_rc6, i915.enable_rc6, int, 0400);
> -MODULE_PARM_DESC(enable_rc6,
> +module_param_named(i915_enable_rc6, i915.enable_rc6, int, 0400);
> +MODULE_PARM_DESC(i915_enable_rc6,
>         "Enable power-saving render C-state 6. "
>         "Different stages can be selected via bitmask values "
>         "(0 = disable; 1 = enable rc6; 2 = enable deep rc6; 4 = enable
> deepest rc6). "
>         "For example, 3 would enable rc6 and deep rc6, and 7 would enable
> everything. "
>         "default: -1 (use per-chip default)");
>
> -module_param_named(enable_fbc, i915.enable_fbc, int, 0600);
> -MODULE_PARM_DESC(enable_fbc,
> +module_param_named(i915_enable_fbc, i915.enable_fbc, int, 0600);
> +MODULE_PARM_DESC(i915_enable_fbc,
>         "Enable frame buffer compression for power savings "
>         "(default: -1 (use per-chip default))");
>
> @@ -111,8 +111,8 @@ MODULE_PARM_DESC(enable_hangcheck,
>         "WARNING: Disabling this can cause system wide hangs. "
>         "(default: true)");
>
> -module_param_named(enable_ppgtt, i915.enable_ppgtt, int, 0400);
> -MODULE_PARM_DESC(enable_ppgtt,
> +module_param_named(i915_enable_ppgtt, i915.enable_ppgtt, int, 0400);
> +MODULE_PARM_DESC(i915_enable_ppgtt,
>         "Override PPGTT usage. "
>         "(-1=auto [default], 0=disabled, 1=aliasing, 2=full)");
>
> --
> 1.9.3
>
>

[-- Attachment #1.2: Type: text/html, Size: 3848 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/1] Revert "drm/i915: drop i915_ prefix from enable_rc6, enable_fbc, enable_ppgtt parameters"

[Daniel Vetter]

On Wed, Jul 16, 2014 at 9:54 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Sorry for the top post, I'm on the road..
>
> In wondering if we couldn't just keep both the old an the new names and have
> them both point at the same variable? Remove the description for the old
> name, but keep it working?

I'm really surprised here ... We have rc6 enabled by default
everywhere, and all the additional rc6 levels that users try to enable
are known to hard-hang machines. I actually have plans to taint the
kernel if you set any of them since I'm fed up with the random crash
reports. Same for fbc, even more so or the ppgtt knob. My stance is
that if you know about these knobs you _really_ should know the driver
to its depths and so also be able to follow module parameter
renamings.

> On Jul 16, 2014 8:34 AM, "Amit Shah" <amit.shah@redhat.com> wrote:
>>
>> This reverts commit 3adee7a7976012a20f1d3b5a529a3c105e29fef1.
>>
>> After upgrading to v3.15, my laptop's battery started draining quite
>> fast.  Powertop pointed to the deep RC6 states not being used.  The
>> kernel param I had put to enable them had stopped working the way it
>> used to; so I disagree with the 'not maintaing ABI' part of the param
>> name change.
>>
>> However weird the names may be, they're in active use and changing them
>> only causes pain for users.  This also isn't advertised (marked
>> deprecated, big warning shown, etc.), so just reverting now.
>>
>> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
>> CC: Jani Nikula <jani.nikula@linux.intel.com>
>> CC: David Airlie <airlied@linux.ie>
>> CC: <stable@vger.kernel.org> # v3.15+
>> Signed-off-by: Amit Shah <amit.shah@redhat.com>

Anyway we need to figure out what went wrong here. Please share your
exact kernelcmdline and lspci -nn. Also stats for before/after from
powertop when idle please.

Thanks, Daniel

>> ---
>>  drivers/gpu/drm/i915/i915_params.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_params.c
>> b/drivers/gpu/drm/i915/i915_params.c
>> index d05a2af..053981d 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -69,16 +69,16 @@ MODULE_PARM_DESC(semaphores,
>>         "Use semaphores for inter-ring sync "
>>         "(default: -1 (use per-chip defaults))");
>>
>> -module_param_named(enable_rc6, i915.enable_rc6, int, 0400);
>> -MODULE_PARM_DESC(enable_rc6,
>> +module_param_named(i915_enable_rc6, i915.enable_rc6, int, 0400);
>> +MODULE_PARM_DESC(i915_enable_rc6,
>>         "Enable power-saving render C-state 6. "
>>         "Different stages can be selected via bitmask values "
>>         "(0 = disable; 1 = enable rc6; 2 = enable deep rc6; 4 = enable
>> deepest rc6). "
>>         "For example, 3 would enable rc6 and deep rc6, and 7 would enable
>> everything. "
>>         "default: -1 (use per-chip default)");
>>
>> -module_param_named(enable_fbc, i915.enable_fbc, int, 0600);
>> -MODULE_PARM_DESC(enable_fbc,
>> +module_param_named(i915_enable_fbc, i915.enable_fbc, int, 0600);
>> +MODULE_PARM_DESC(i915_enable_fbc,
>>         "Enable frame buffer compression for power savings "
>>         "(default: -1 (use per-chip default))");
>>
>> @@ -111,8 +111,8 @@ MODULE_PARM_DESC(enable_hangcheck,
>>         "WARNING: Disabling this can cause system wide hangs. "
>>         "(default: true)");
>>
>> -module_param_named(enable_ppgtt, i915.enable_ppgtt, int, 0400);
>> -MODULE_PARM_DESC(enable_ppgtt,
>> +module_param_named(i915_enable_ppgtt, i915.enable_ppgtt, int, 0400);
>> +MODULE_PARM_DESC(i915_enable_ppgtt,
>>         "Override PPGTT usage. "
>>         "(-1=auto [default], 0=disabled, 1=aliasing, 2=full)");
>>
>> --
>> 1.9.3
>>
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/1] Revert "drm/i915: drop i915_ prefix from enable_rc6, enable_fbc, enable_ppgtt parameters"

[Amit Shah]

On (Thu) 17 Jul 2014 [09:35:20], Daniel Vetter wrote:
> On Wed, Jul 16, 2014 at 9:54 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > Sorry for the top post, I'm on the road..
> >
> > In wondering if we couldn't just keep both the old an the new names and have
> > them both point at the same variable? Remove the description for the old
> > name, but keep it working?
> 
> I'm really surprised here ... We have rc6 enabled by default
> everywhere, and all the additional rc6 levels that users try to enable
> are known to hard-hang machines.

I haven't had this problem on my hardware (ThinkPad T420s, lspci
below) for a few kernel versions.  I think I added the enable_rc6=
setting back from the time the deeper states were enabled and then
reverted for SandyBridge.

Nevertheless, with the current state, RC6p and RC6pp states are not
used.

> I actually have plans to taint the
> kernel if you set any of them since I'm fed up with the random crash
> reports. Same for fbc, even more so or the ppgtt knob. My stance is
> that if you know about these knobs you _really_ should know the driver
> to its depths and so also be able to follow module parameter
> renamings.

I also remember there being bugzillas about power consumption, and
using this setting was recommended (for Fedora, I think).  I know
a few people are using this setting.

> > On Jul 16, 2014 8:34 AM, "Amit Shah" <amit.shah@redhat.com> wrote:
> >>
> >> This reverts commit 3adee7a7976012a20f1d3b5a529a3c105e29fef1.
> >>
> >> After upgrading to v3.15, my laptop's battery started draining quite
> >> fast.  Powertop pointed to the deep RC6 states not being used.  The
> >> kernel param I had put to enable them had stopped working the way it
> >> used to; so I disagree with the 'not maintaing ABI' part of the param
> >> name change.
> >>
> >> However weird the names may be, they're in active use and changing them
> >> only causes pain for users.  This also isn't advertised (marked
> >> deprecated, big warning shown, etc.), so just reverting now.
> >>
> >> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> CC: Jani Nikula <jani.nikula@linux.intel.com>
> >> CC: David Airlie <airlied@linux.ie>
> >> CC: <stable@vger.kernel.org> # v3.15+
> >> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> 
> Anyway we need to figure out what went wrong here. Please share your
> exact kernelcmdline and lspci -nn. Also stats for before/after from
> powertop when idle please.

Powertop stats for idle are a little difficult -- since this is my
primary laptop.

BOOT_IMAGE=/vmlinuz-3.15.4-200.fc20.x86_64 root=/dev/mapper/luks-3aff2acf-737d-4002-b644-15f599d09a18 ro rd.lvm.lv=fedora_grmbl/00 rd.lvm.lv=fedora_grmbl/01 vconsole.font=latarcyrheb-sun16 rd.luks.uuid=luks-0934d354-5b07-4e91-a699-9bfc57e76fdc rd.luks.uuid=luks-3aff2acf-737d-4002-b644-15f599d09a18 rhgb quiet slub_debug=- i915.i915_enable_rc6=7 LANG=en_IN.UTF-8

00:00.0 Host bridge [0600]: Intel Corporation 2nd Generation Core Processor Family DRAM Controller [8086:0104] (rev 09)
00:02.0 VGA compatible controller [0300]: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller [8086:0126] (rev 09)
00:16.0 Communication controller [0780]: Intel Corporation 6 Series/C200 Series Chipset Family MEI Controller #1 [8086:1c3a] (rev 04)
00:19.0 Ethernet controller [0200]: Intel Corporation 82579LM Gigabit Network Connection [8086:1502] (rev 04)
00:1a.0 USB controller [0c03]: Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #2 [8086:1c2d] (rev 04)
00:1b.0 Audio device [0403]: Intel Corporation 6 Series/C200 Series Chipset Family High Definition Audio Controller [8086:1c20] (rev 04)
00:1c.0 PCI bridge [0604]: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 1 [8086:1c10] (rev b4)
00:1c.1 PCI bridge [0604]: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 2 [8086:1c12] (rev b4)
00:1c.3 PCI bridge [0604]: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 4 [8086:1c16] (rev b4)
00:1c.4 PCI bridge [0604]: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 5 [8086:1c18] (rev b4)
00:1d.0 USB controller [0c03]: Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #1 [8086:1c26] (rev 04)
00:1f.0 ISA bridge [0601]: Intel Corporation QM67 Express Chipset Family LPC Controller [8086:1c4f] (rev 04)
00:1f.2 SATA controller [0106]: Intel Corporation 6 Series/C200 Series Chipset Family 6 port SATA AHCI Controller [8086:1c03] (rev 04)
00:1f.3 SMBus [0c05]: Intel Corporation 6 Series/C200 Series Chipset Family SMBus Controller [8086:1c22] (rev 04)
03:00.0 Network controller [0280]: Intel Corporation Centrino Advanced-N 6205 [Taylor Peak] [8086:0085] (rev 34)
05:00.0 SD Host controller [0805]: Ricoh Co Ltd MMC/SD Host Controller [1180:e822] (rev 07)
0d:00.0 USB controller [0c03]: NEC Corporation uPD720200 USB 3.0 Host Controller [1033:0194] (rev 04)


Thanks,
		Amit

Re: [PATCH 1/1] Revert "drm/i915: drop i915_ prefix from enable_rc6, enable_fbc, enable_ppgtt parameters"

[Daniel Vetter]

On Thu, Jul 17, 2014 at 02:32:41PM +0530, Amit Shah wrote:
> On (Thu) 17 Jul 2014 [09:35:20], Daniel Vetter wrote:
> > On Wed, Jul 16, 2014 at 9:54 PM, Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> > > Sorry for the top post, I'm on the road..
> > >
> > > In wondering if we couldn't just keep both the old an the new names and have
> > > them both point at the same variable? Remove the description for the old
> > > name, but keep it working?
> > 
> > I'm really surprised here ... We have rc6 enabled by default
> > everywhere, and all the additional rc6 levels that users try to enable
> > are known to hard-hang machines.
> 
> I haven't had this problem on my hardware (ThinkPad T420s, lspci
> below) for a few kernel versions.  I think I added the enable_rc6=
> setting back from the time the deeper states were enabled and then
> reverted for SandyBridge.
> 
> Nevertheless, with the current state, RC6p and RC6pp states are not
> used.

Yeah, on snb they cause crashes and instability and also don't provide
measurable power benefits (afaik). So I recommend you drop that one.

> > I actually have plans to taint the
> > kernel if you set any of them since I'm fed up with the random crash
> > reports. Same for fbc, even more so or the ppgtt knob. My stance is
> > that if you know about these knobs you _really_ should know the driver
> > to its depths and so also be able to follow module parameter
> > renamings.
> 
> I also remember there being bugzillas about power consumption, and
> using this setting was recommended (for Fedora, I think).  I know
> a few people are using this setting.

I know, google is littered with such entries. Unfortunately by the time
google thinks something is important (which usually takes a few months)
it's already badly outdated: i915 graphics developement is charging ahead
at a really brisk pace - we merge a few hundred patches per release for
i915 alone.
 
> > > On Jul 16, 2014 8:34 AM, "Amit Shah" <amit.shah@redhat.com> wrote:
> > >>
> > >> This reverts commit 3adee7a7976012a20f1d3b5a529a3c105e29fef1.
> > >>
> > >> After upgrading to v3.15, my laptop's battery started draining quite
> > >> fast.  Powertop pointed to the deep RC6 states not being used.  The
> > >> kernel param I had put to enable them had stopped working the way it
> > >> used to; so I disagree with the 'not maintaing ABI' part of the param
> > >> name change.
> > >>
> > >> However weird the names may be, they're in active use and changing them
> > >> only causes pain for users.  This also isn't advertised (marked
> > >> deprecated, big warning shown, etc.), so just reverting now.
> > >>
> > >> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >> CC: Jani Nikula <jani.nikula@linux.intel.com>
> > >> CC: David Airlie <airlied@linux.ie>
> > >> CC: <stable@vger.kernel.org> # v3.15+
> > >> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > 
> > Anyway we need to figure out what went wrong here. Please share your
> > exact kernelcmdline and lspci -nn. Also stats for before/after from
> > powertop when idle please.
> 
> Powertop stats for idle are a little difficult -- since this is my
> primary laptop.

Now I'm a bit confused: How have you measured that the lack of rc6p/pp is
the reason for your power consumption regression?
-Daniel

> 
> BOOT_IMAGE=/vmlinuz-3.15.4-200.fc20.x86_64 root=/dev/mapper/luks-3aff2acf-737d-4002-b644-15f599d09a18 ro rd.lvm.lv=fedora_grmbl/00 rd.lvm.lv=fedora_grmbl/01 vconsole.font=latarcyrheb-sun16 rd.luks.uuid=luks-0934d354-5b07-4e91-a699-9bfc57e76fdc rd.luks.uuid=luks-3aff2acf-737d-4002-b644-15f599d09a18 rhgb quiet slub_debug=- i915.i915_enable_rc6=7 LANG=en_IN.UTF-8
> 
> 00:00.0 Host bridge [0600]: Intel Corporation 2nd Generation Core Processor Family DRAM Controller [8086:0104] (rev 09)
> 00:02.0 VGA compatible controller [0300]: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller [8086:0126] (rev 09)
> 00:16.0 Communication controller [0780]: Intel Corporation 6 Series/C200 Series Chipset Family MEI Controller #1 [8086:1c3a] (rev 04)
> 00:19.0 Ethernet controller [0200]: Intel Corporation 82579LM Gigabit Network Connection [8086:1502] (rev 04)
> 00:1a.0 USB controller [0c03]: Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #2 [8086:1c2d] (rev 04)
> 00:1b.0 Audio device [0403]: Intel Corporation 6 Series/C200 Series Chipset Family High Definition Audio Controller [8086:1c20] (rev 04)
> 00:1c.0 PCI bridge [0604]: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 1 [8086:1c10] (rev b4)
> 00:1c.1 PCI bridge [0604]: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 2 [8086:1c12] (rev b4)
> 00:1c.3 PCI bridge [0604]: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 4 [8086:1c16] (rev b4)
> 00:1c.4 PCI bridge [0604]: Intel Corporation 6 Series/C200 Series Chipset Family PCI Express Root Port 5 [8086:1c18] (rev b4)
> 00:1d.0 USB controller [0c03]: Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #1 [8086:1c26] (rev 04)
> 00:1f.0 ISA bridge [0601]: Intel Corporation QM67 Express Chipset Family LPC Controller [8086:1c4f] (rev 04)
> 00:1f.2 SATA controller [0106]: Intel Corporation 6 Series/C200 Series Chipset Family 6 port SATA AHCI Controller [8086:1c03] (rev 04)
> 00:1f.3 SMBus [0c05]: Intel Corporation 6 Series/C200 Series Chipset Family SMBus Controller [8086:1c22] (rev 04)
> 03:00.0 Network controller [0280]: Intel Corporation Centrino Advanced-N 6205 [Taylor Peak] [8086:0085] (rev 34)
> 05:00.0 SD Host controller [0805]: Ricoh Co Ltd MMC/SD Host Controller [1180:e822] (rev 07)
> 0d:00.0 USB controller [0c03]: NEC Corporation uPD720200 USB 3.0 Host Controller [1033:0194] (rev 04)
> 
> 
> Thanks,
> 		Amit

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/1] Revert "drm/i915: drop i915_ prefix from enable_rc6, enable_fbc, enable_ppgtt parameters"

[Amit Shah]

On (Thu) 17 Jul 2014 [11:11:15], Daniel Vetter wrote:
> On Thu, Jul 17, 2014 at 02:32:41PM +0530, Amit Shah wrote:
> > On (Thu) 17 Jul 2014 [09:35:20], Daniel Vetter wrote:
> > > On Wed, Jul 16, 2014 at 9:54 PM, Linus Torvalds
> > > <torvalds@linux-foundation.org> wrote:
> > > > Sorry for the top post, I'm on the road..
> > > >
> > > > In wondering if we couldn't just keep both the old an the new names and have
> > > > them both point at the same variable? Remove the description for the old
> > > > name, but keep it working?
> > > 
> > > I'm really surprised here ... We have rc6 enabled by default
> > > everywhere, and all the additional rc6 levels that users try to enable
> > > are known to hard-hang machines.
> > 
> > I haven't had this problem on my hardware (ThinkPad T420s, lspci
> > below) for a few kernel versions.  I think I added the enable_rc6=
> > setting back from the time the deeper states were enabled and then
> > reverted for SandyBridge.
> > 
> > Nevertheless, with the current state, RC6p and RC6pp states are not
> > used.
> 
> Yeah, on snb they cause crashes and instability and also don't provide
> measurable power benefits (afaik). So I recommend you drop that one.

Not for me -- there have been no crashes / hangs / lockups as I
mentioned.

> > > I actually have plans to taint the
> > > kernel if you set any of them since I'm fed up with the random crash
> > > reports. Same for fbc, even more so or the ppgtt knob. My stance is
> > > that if you know about these knobs you _really_ should know the driver
> > > to its depths and so also be able to follow module parameter
> > > renamings.
> > 
> > I also remember there being bugzillas about power consumption, and
> > using this setting was recommended (for Fedora, I think).  I know
> > a few people are using this setting.
> 
> I know, google is littered with such entries. Unfortunately by the time
> google thinks something is important (which usually takes a few months)
> it's already badly outdated: i915 graphics developement is charging ahead
> at a really brisk pace - we merge a few hundred patches per release for
> i915 alone.

But for SNB, there's really no "improvement" for the RC6 states, is
there?

> > > > On Jul 16, 2014 8:34 AM, "Amit Shah" <amit.shah@redhat.com> wrote:
> > > >>
> > > >> This reverts commit 3adee7a7976012a20f1d3b5a529a3c105e29fef1.
> > > >>
> > > >> After upgrading to v3.15, my laptop's battery started draining quite
> > > >> fast.  Powertop pointed to the deep RC6 states not being used.  The
> > > >> kernel param I had put to enable them had stopped working the way it
> > > >> used to; so I disagree with the 'not maintaing ABI' part of the param
> > > >> name change.
> > > >>
> > > >> However weird the names may be, they're in active use and changing them
> > > >> only causes pain for users.  This also isn't advertised (marked
> > > >> deprecated, big warning shown, etc.), so just reverting now.
> > > >>
> > > >> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > >> CC: Jani Nikula <jani.nikula@linux.intel.com>
> > > >> CC: David Airlie <airlied@linux.ie>
> > > >> CC: <stable@vger.kernel.org> # v3.15+
> > > >> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > > 
> > > Anyway we need to figure out what went wrong here. Please share your
> > > exact kernelcmdline and lspci -nn. Also stats for before/after from
> > > powertop when idle please.
> > 
> > Powertop stats for idle are a little difficult -- since this is my
> > primary laptop.
> 
> Now I'm a bit confused: How have you measured that the lack of rc6p/pp is
> the reason for your power consumption regression?
> -Daniel

What I meant was rebooting in the middle of something is a pain
(usually a week or two between trying these things); and also for a
fair comparison, the workloads have to be similar for both the
powertop ratings.

In any case, my daily work doesn't change, and I noticed this
immediately upon booting into 3.15.  The laptop heats up a bit more,
that's the first clue; and the battery doesn't provide as much backup
as it used to.

		Amit

Re: [PATCH 1/1] Revert "drm/i915: drop i915_ prefix from enable_rc6, enable_fbc, enable_ppgtt parameters"

[Amit Shah]

[-- Attachment #1: Type: text/plain, Size: 4827 bytes --]

On (Thu) 17 Jul 2014 [14:50:12], Amit Shah wrote:
> On (Thu) 17 Jul 2014 [11:11:15], Daniel Vetter wrote:
> > On Thu, Jul 17, 2014 at 02:32:41PM +0530, Amit Shah wrote:
> > > On (Thu) 17 Jul 2014 [09:35:20], Daniel Vetter wrote:
> > > > On Wed, Jul 16, 2014 at 9:54 PM, Linus Torvalds
> > > > <torvalds@linux-foundation.org> wrote:
> > > > > Sorry for the top post, I'm on the road..
> > > > >
> > > > > In wondering if we couldn't just keep both the old an the new names and have
> > > > > them both point at the same variable? Remove the description for the old
> > > > > name, but keep it working?
> > > > 
> > > > I'm really surprised here ... We have rc6 enabled by default
> > > > everywhere, and all the additional rc6 levels that users try to enable
> > > > are known to hard-hang machines.
> > > 
> > > I haven't had this problem on my hardware (ThinkPad T420s, lspci
> > > below) for a few kernel versions.  I think I added the enable_rc6=
> > > setting back from the time the deeper states were enabled and then
> > > reverted for SandyBridge.
> > > 
> > > Nevertheless, with the current state, RC6p and RC6pp states are not
> > > used.
> > 
> > Yeah, on snb they cause crashes and instability and also don't provide
> > measurable power benefits (afaik). So I recommend you drop that one.
> 
> Not for me -- there have been no crashes / hangs / lockups as I
> mentioned.
> 
> > > > I actually have plans to taint the
> > > > kernel if you set any of them since I'm fed up with the random crash
> > > > reports. Same for fbc, even more so or the ppgtt knob. My stance is
> > > > that if you know about these knobs you _really_ should know the driver
> > > > to its depths and so also be able to follow module parameter
> > > > renamings.
> > > 
> > > I also remember there being bugzillas about power consumption, and
> > > using this setting was recommended (for Fedora, I think).  I know
> > > a few people are using this setting.
> > 
> > I know, google is littered with such entries. Unfortunately by the time
> > google thinks something is important (which usually takes a few months)
> > it's already badly outdated: i915 graphics developement is charging ahead
> > at a really brisk pace - we merge a few hundred patches per release for
> > i915 alone.
> 
> But for SNB, there's really no "improvement" for the RC6 states, is
> there?
> 
> > > > > On Jul 16, 2014 8:34 AM, "Amit Shah" <amit.shah@redhat.com> wrote:
> > > > >>
> > > > >> This reverts commit 3adee7a7976012a20f1d3b5a529a3c105e29fef1.
> > > > >>
> > > > >> After upgrading to v3.15, my laptop's battery started draining quite
> > > > >> fast.  Powertop pointed to the deep RC6 states not being used.  The
> > > > >> kernel param I had put to enable them had stopped working the way it
> > > > >> used to; so I disagree with the 'not maintaing ABI' part of the param
> > > > >> name change.
> > > > >>
> > > > >> However weird the names may be, they're in active use and changing them
> > > > >> only causes pain for users.  This also isn't advertised (marked
> > > > >> deprecated, big warning shown, etc.), so just reverting now.
> > > > >>
> > > > >> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > >> CC: Jani Nikula <jani.nikula@linux.intel.com>
> > > > >> CC: David Airlie <airlied@linux.ie>
> > > > >> CC: <stable@vger.kernel.org> # v3.15+
> > > > >> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > > > 
> > > > Anyway we need to figure out what went wrong here. Please share your
> > > > exact kernelcmdline and lspci -nn. Also stats for before/after from
> > > > powertop when idle please.
> > > 
> > > Powertop stats for idle are a little difficult -- since this is my
> > > primary laptop.
> > 
> > Now I'm a bit confused: How have you measured that the lack of rc6p/pp is
> > the reason for your power consumption regression?
> > -Daniel
> 
> What I meant was rebooting in the middle of something is a pain
> (usually a week or two between trying these things); and also for a
> fair comparison, the workloads have to be similar for both the
> powertop ratings.

Attached are the two powertop runs with 'powertop --html'.  Both are
taken on a fresh reboot on the same kernel, just the difference is
that powertop-rc6pp.html has i915.enable_rc6=7 in the cmdline.  Dunno
if that's what you wanted, but it shows the laptop fan is spinning
with more rpm, fwiw.  Both runs are taken after about 10 mins of
desktop idling.


> In any case, my daily work doesn't change, and I noticed this
> immediately upon booting into 3.15.  The laptop heats up a bit more,
> that's the first clue; and the battery doesn't provide as much backup
> as it used to.

As I stated, I've used the deeper states on this h/w for a while w/o
any adverse effects.  So please consider picking this revert, or
enable the deeper states for this h/w.



		Amit

[-- Attachment #2: powertop-rc6pp.html --]
[-- Type: text/html, Size: 57891 bytes --]

[-- Attachment #3: powertop-norc6.html --]
[-- Type: text/html, Size: 53393 bytes --]

[-- Attachment #4: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx