[Intel-gfx] [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 at 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 at 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 at ffwll.ch>
>> CC: Jani Nikula <jani.nikula at linux.intel.com>
>> CC: David Airlie <airlied at linux.ie>
>> CC: <stable at vger.kernel.org> # v3.15+
>> Signed-off-by: Amit Shah <amit.shah at 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