Re: [Intel-gfx] [PATCH 0197/1285] Replace numeric parameter like 0444 with macro

[Lukas Wunner]

On Tue, Aug 02, 2016 at 02:37:37PM +0300, Ville Syrjälä wrote:
> On Tue, Aug 02, 2016 at 06:48:47PM +0800, Baole Ni wrote:
> > I find that the developers often just specified the numeric value
> > when calling a macro which is defined with a parameter for access 
> > permission.
> > As we know, these numeric value for access permission have had the 
> > corresponding macro,
> > and that using macro can improve the robustness and readability of the code,
> > thus, I suggest replacing the numeric parameter with the macro.
> > 
> > Signed-off-by: Chuansheng Liu 
> > Signed-off-by: Baole Ni 
> > ---
> >  drivers/gpu/drm/i915/i915_params.c | 64 
> > +++---
> >  1 file changed, 32 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_params.c 
> > b/drivers/gpu/drm/i915/i915_params.c
> > index 1779f02..7184e06 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -60,22 +60,22 @@ struct i915_params i915 __read_mostly = {
> > .inject_load_failure = 0,
> >  };
> >  
> > -module_param_named(modeset, i915.modeset, int, 0400);
> > +module_param_named(modeset, i915.modeset, int, S_IRUSR);
> 
> At least I can't read those macros. Octal is much clearer IMO.

It's also easier to grep for, say, 644, rather than formulating a
regex with all possible ordering permutations of these macros.

Best regards,

Lukas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0197/1285] Replace numeric parameter like 0444 with macro

[Daniel Vetter]

On Tue, Aug 02, 2016 at 02:37:37PM +0300, Ville Syrjälä wrote:
> On Tue, Aug 02, 2016 at 06:48:47PM +0800, Baole Ni wrote:
> > I find that the developers often just specified the numeric value
> > when calling a macro which is defined with a parameter for access 
> > permission.
> > As we know, these numeric value for access permission have had the 
> > corresponding macro,
> > and that using macro can improve the robustness and readability of the code,
> > thus, I suggest replacing the numeric parameter with the macro.
> > 
> > Signed-off-by: Chuansheng Liu 
> > Signed-off-by: Baole Ni 
> > ---
> >  drivers/gpu/drm/i915/i915_params.c | 64 
> > +++---
> >  1 file changed, 32 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_params.c 
> > b/drivers/gpu/drm/i915/i915_params.c
> > index 1779f02..7184e06 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -60,22 +60,22 @@ struct i915_params i915 __read_mostly = {
> > .inject_load_failure = 0,
> >  };
> >  
> > -module_param_named(modeset, i915.modeset, int, 0400);
> > +module_param_named(modeset, i915.modeset, int, S_IRUSR);
> 
> At least I can't read those macros. Octal is much clearer IMO.

Yeah, not really sold on this either.
-Daniel

> 
> >  MODULE_PARM_DESC(modeset,
> > "Use kernel modesetting [KMS] (0=disable, "
> > "1=on, -1=force vga console preference [default])");
> >  
> > -module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 
> > 0600);
> > +module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 
> > S_IRUSR | S_IWUSR);
> >  MODULE_PARM_DESC(panel_ignore_lid,
> > "Override lid status (0=autodetect, 1=autodetect disabled [default], "
> > "-1=force lid closed, -2=force lid open)");
> >  
> > -module_param_named_unsafe(semaphores, i915.semaphores, int, 0400);
> > +module_param_named_unsafe(semaphores, i915.semaphores, int, S_IRUSR);
> >  MODULE_PARM_DESC(semaphores,
> > "Use semaphores for inter-ring sync "
> > "(default: -1 (use per-chip defaults))");
> >  
> > -module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, 0400);
> > +module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, S_IRUSR);
> >  MODULE_PARM_DESC(enable_rc6,
> > "Enable power-saving render C-state 6. "
> > "Different stages can be selected via bitmask values "
> > @@ -83,82 +83,82 @@ MODULE_PARM_DESC(enable_rc6,
> > "For example, 3 would enable rc6 and deep rc6, and 7 would enable 
> > everything. "
> > "default: -1 (use per-chip default)");
> >  
> > -module_param_named_unsafe(enable_dc, i915.enable_dc, int, 0400);
> > +module_param_named_unsafe(enable_dc, i915.enable_dc, int, S_IRUSR);
> >  MODULE_PARM_DESC(enable_dc,
> > "Enable power-saving display C-states. "
> > "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
> >  
> > -module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
> > +module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, S_IRUSR | 
> > S_IWUSR);
> >  MODULE_PARM_DESC(enable_fbc,
> > "Enable frame buffer compression for power savings "
> > "(default: -1 (use per-chip default))");
> >  
> > -module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 
> > 0400);
> > +module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 
> > S_IRUSR);
> >  MODULE_PARM_DESC(lvds_channel_mode,
> >  "Specify LVDS channel mode "
> >  "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)");
> >  
> > -module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, 0600);
> > +module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, S_IRUSR | 
> > S_IWUSR);
> >  MODULE_PARM_DESC(lvds_use_ssc,
> > "Use Spread Spectrum Clock with panels [LVDS/eDP] "
> > "(default: auto from VBT)");
> >  
> > -module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, 
> > int, 0400);
> > +module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, 
> > int, S_IRUSR);
> >  MODULE_PARM_DESC(vbt_sdvo_panel_type,
> > "Override/Ignore selection of SDVO panel mode in the VBT "
> > "(-2=ignore, -1=auto [default], index in VBT BIOS table)");
> >  
> > -module_param_named_unsafe(reset, i915.reset, bool, 0600);
> > +module_param_named_unsafe(reset, i915.reset, bool, S_IRUSR | S_IWUSR);
> >  MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
> >  
> > -module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 
> > 0644);
> > +module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 
> > S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> >  MODULE_PARM_DESC(enable_hangcheck,
> > "Periodically check GPU activity for detecting hangs. "
> > "WARNING: Disabling this can cause system wide hangs. "
> > "(default: true)");
> >  
> > -module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, 

Re: [Intel-gfx] [PATCH 0197/1285] Replace numeric parameter like 0444 with macro

[Jani Nikula]

On Tue, 02 Aug 2016, Ville Syrjälä  wrote:
> On Tue, Aug 02, 2016 at 06:48:47PM +0800, Baole Ni wrote:
>> I find that the developers often just specified the numeric value
>> when calling a macro which is defined with a parameter for access permission.
>> As we know, these numeric value for access permission have had the 
>> corresponding macro,
>> and that using macro can improve the robustness and readability of the code,
>> thus, I suggest replacing the numeric parameter with the macro.
>> 
>> Signed-off-by: Chuansheng Liu 
>> Signed-off-by: Baole Ni 
>> ---
>>  drivers/gpu/drm/i915/i915_params.c | 64 
>> +++---
>>  1 file changed, 32 insertions(+), 32 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_params.c 
>> b/drivers/gpu/drm/i915/i915_params.c
>> index 1779f02..7184e06 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -60,22 +60,22 @@ struct i915_params i915 __read_mostly = {
>>  .inject_load_failure = 0,
>>  };
>>  
>> -module_param_named(modeset, i915.modeset, int, 0400);
>> +module_param_named(modeset, i915.modeset, int, S_IRUSR);
>
> At least I can't read those macros. Octal is much clearer IMO.

Besides I think we should consider making most if not all of these
parameters group/other readable. Which would then potentially become
S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH instead of just 0644.

BR,
Jani.

>
>>  MODULE_PARM_DESC(modeset,
>>  "Use kernel modesetting [KMS] (0=disable, "
>>  "1=on, -1=force vga console preference [default])");
>>  
>> -module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 
>> 0600);
>> +module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 
>> S_IRUSR | S_IWUSR);
>>  MODULE_PARM_DESC(panel_ignore_lid,
>>  "Override lid status (0=autodetect, 1=autodetect disabled [default], "
>>  "-1=force lid closed, -2=force lid open)");
>>  
>> -module_param_named_unsafe(semaphores, i915.semaphores, int, 0400);
>> +module_param_named_unsafe(semaphores, i915.semaphores, int, S_IRUSR);
>>  MODULE_PARM_DESC(semaphores,
>>  "Use semaphores for inter-ring sync "
>>  "(default: -1 (use per-chip defaults))");
>>  
>> -module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, 0400);
>> +module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, S_IRUSR);
>>  MODULE_PARM_DESC(enable_rc6,
>>  "Enable power-saving render C-state 6. "
>>  "Different stages can be selected via bitmask values "
>> @@ -83,82 +83,82 @@ MODULE_PARM_DESC(enable_rc6,
>>  "For example, 3 would enable rc6 and deep rc6, and 7 would enable 
>> everything. "
>>  "default: -1 (use per-chip default)");
>>  
>> -module_param_named_unsafe(enable_dc, i915.enable_dc, int, 0400);
>> +module_param_named_unsafe(enable_dc, i915.enable_dc, int, S_IRUSR);
>>  MODULE_PARM_DESC(enable_dc,
>>  "Enable power-saving display C-states. "
>>  "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
>>  
>> -module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
>> +module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, S_IRUSR | 
>> S_IWUSR);
>>  MODULE_PARM_DESC(enable_fbc,
>>  "Enable frame buffer compression for power savings "
>>  "(default: -1 (use per-chip default))");
>>  
>> -module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 
>> 0400);
>> +module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 
>> S_IRUSR);
>>  MODULE_PARM_DESC(lvds_channel_mode,
>>   "Specify LVDS channel mode "
>>   "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)");
>>  
>> -module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, 0600);
>> +module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, S_IRUSR | 
>> S_IWUSR);
>>  MODULE_PARM_DESC(lvds_use_ssc,
>>  "Use Spread Spectrum Clock with panels [LVDS/eDP] "
>>  "(default: auto from VBT)");
>>  
>> -module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, 
>> int, 0400);
>> +module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, 
>> int, S_IRUSR);
>>  MODULE_PARM_DESC(vbt_sdvo_panel_type,
>>  "Override/Ignore selection of SDVO panel mode in the VBT "
>>  "(-2=ignore, -1=auto [default], index in VBT BIOS table)");
>>  
>> -module_param_named_unsafe(reset, i915.reset, bool, 0600);
>> +module_param_named_unsafe(reset, i915.reset, bool, S_IRUSR | S_IWUSR);
>>  MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
>>  
>> -module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 
>> 0644);
>> +module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 
>> S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
>>  MODULE_PARM_DESC(enable_hangcheck,
>>  "Periodically check GPU activity for detecting hangs. "
>>  "WARNING: Disabling this can cause system wide hangs. "
>>  

Re: [Intel-gfx] [PATCH 0197/1285] Replace numeric parameter like 0444 with macro

[Ville Syrjälä]

On Tue, Aug 02, 2016 at 06:48:47PM +0800, Baole Ni wrote:
> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the 
> corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
> 
> Signed-off-by: Chuansheng Liu 
> Signed-off-by: Baole Ni 
> ---
>  drivers/gpu/drm/i915/i915_params.c | 64 
> +++---
>  1 file changed, 32 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c 
> b/drivers/gpu/drm/i915/i915_params.c
> index 1779f02..7184e06 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -60,22 +60,22 @@ struct i915_params i915 __read_mostly = {
>   .inject_load_failure = 0,
>  };
>  
> -module_param_named(modeset, i915.modeset, int, 0400);
> +module_param_named(modeset, i915.modeset, int, S_IRUSR);

At least I can't read those macros. Octal is much clearer IMO.

>  MODULE_PARM_DESC(modeset,
>   "Use kernel modesetting [KMS] (0=disable, "
>   "1=on, -1=force vga console preference [default])");
>  
> -module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 
> 0600);
> +module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 
> S_IRUSR | S_IWUSR);
>  MODULE_PARM_DESC(panel_ignore_lid,
>   "Override lid status (0=autodetect, 1=autodetect disabled [default], "
>   "-1=force lid closed, -2=force lid open)");
>  
> -module_param_named_unsafe(semaphores, i915.semaphores, int, 0400);
> +module_param_named_unsafe(semaphores, i915.semaphores, int, S_IRUSR);
>  MODULE_PARM_DESC(semaphores,
>   "Use semaphores for inter-ring sync "
>   "(default: -1 (use per-chip defaults))");
>  
> -module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, 0400);
> +module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, S_IRUSR);
>  MODULE_PARM_DESC(enable_rc6,
>   "Enable power-saving render C-state 6. "
>   "Different stages can be selected via bitmask values "
> @@ -83,82 +83,82 @@ MODULE_PARM_DESC(enable_rc6,
>   "For example, 3 would enable rc6 and deep rc6, and 7 would enable 
> everything. "
>   "default: -1 (use per-chip default)");
>  
> -module_param_named_unsafe(enable_dc, i915.enable_dc, int, 0400);
> +module_param_named_unsafe(enable_dc, i915.enable_dc, int, S_IRUSR);
>  MODULE_PARM_DESC(enable_dc,
>   "Enable power-saving display C-states. "
>   "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
>  
> -module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
> +module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, S_IRUSR | 
> S_IWUSR);
>  MODULE_PARM_DESC(enable_fbc,
>   "Enable frame buffer compression for power savings "
>   "(default: -1 (use per-chip default))");
>  
> -module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 
> 0400);
> +module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 
> S_IRUSR);
>  MODULE_PARM_DESC(lvds_channel_mode,
>"Specify LVDS channel mode "
>"(0=probe BIOS [default], 1=single-channel, 2=dual-channel)");
>  
> -module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, 0600);
> +module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, S_IRUSR | 
> S_IWUSR);
>  MODULE_PARM_DESC(lvds_use_ssc,
>   "Use Spread Spectrum Clock with panels [LVDS/eDP] "
>   "(default: auto from VBT)");
>  
> -module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, 
> int, 0400);
> +module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, 
> int, S_IRUSR);
>  MODULE_PARM_DESC(vbt_sdvo_panel_type,
>   "Override/Ignore selection of SDVO panel mode in the VBT "
>   "(-2=ignore, -1=auto [default], index in VBT BIOS table)");
>  
> -module_param_named_unsafe(reset, i915.reset, bool, 0600);
> +module_param_named_unsafe(reset, i915.reset, bool, S_IRUSR | S_IWUSR);
>  MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
>  
> -module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 
> 0644);
> +module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 
> S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
>  MODULE_PARM_DESC(enable_hangcheck,
>   "Periodically check GPU activity for detecting hangs. "
>   "WARNING: Disabling this can cause system wide hangs. "
>   "(default: true)");
>  
> -module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, 0400);
> +module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, S_IRUSR);
>  MODULE_PARM_DESC(enable_ppgtt,
>   "Override PPGTT usage. "
>   "(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with 
> extended address space)");
>  
> 

[Intel-gfx] [PATCH 0197/1285] Replace numeric parameter like 0444 with macro

[Baole Ni]

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the 
corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu 
Signed-off-by: Baole Ni 
---
 drivers/gpu/drm/i915/i915_params.c | 64 +++---
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index 1779f02..7184e06 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -60,22 +60,22 @@ struct i915_params i915 __read_mostly = {
.inject_load_failure = 0,
 };
 
-module_param_named(modeset, i915.modeset, int, 0400);
+module_param_named(modeset, i915.modeset, int, S_IRUSR);
 MODULE_PARM_DESC(modeset,
"Use kernel modesetting [KMS] (0=disable, "
"1=on, -1=force vga console preference [default])");
 
-module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 0600);
+module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 
S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(panel_ignore_lid,
"Override lid status (0=autodetect, 1=autodetect disabled [default], "
"-1=force lid closed, -2=force lid open)");
 
-module_param_named_unsafe(semaphores, i915.semaphores, int, 0400);
+module_param_named_unsafe(semaphores, i915.semaphores, int, S_IRUSR);
 MODULE_PARM_DESC(semaphores,
"Use semaphores for inter-ring sync "
"(default: -1 (use per-chip defaults))");
 
-module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, 0400);
+module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, S_IRUSR);
 MODULE_PARM_DESC(enable_rc6,
"Enable power-saving render C-state 6. "
"Different stages can be selected via bitmask values "
@@ -83,82 +83,82 @@ MODULE_PARM_DESC(enable_rc6,
"For example, 3 would enable rc6 and deep rc6, and 7 would enable 
everything. "
"default: -1 (use per-chip default)");
 
-module_param_named_unsafe(enable_dc, i915.enable_dc, int, 0400);
+module_param_named_unsafe(enable_dc, i915.enable_dc, int, S_IRUSR);
 MODULE_PARM_DESC(enable_dc,
"Enable power-saving display C-states. "
"(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
 
-module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
+module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(enable_fbc,
"Enable frame buffer compression for power savings "
"(default: -1 (use per-chip default))");
 
-module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 
0400);
+module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 
S_IRUSR);
 MODULE_PARM_DESC(lvds_channel_mode,
 "Specify LVDS channel mode "
 "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)");
 
-module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, 0600);
+module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, S_IRUSR | 
S_IWUSR);
 MODULE_PARM_DESC(lvds_use_ssc,
"Use Spread Spectrum Clock with panels [LVDS/eDP] "
"(default: auto from VBT)");
 
-module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, 
0400);
+module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, 
S_IRUSR);
 MODULE_PARM_DESC(vbt_sdvo_panel_type,
"Override/Ignore selection of SDVO panel mode in the VBT "
"(-2=ignore, -1=auto [default], index in VBT BIOS table)");
 
-module_param_named_unsafe(reset, i915.reset, bool, 0600);
+module_param_named_unsafe(reset, i915.reset, bool, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
 
-module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 0644);
+module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
 MODULE_PARM_DESC(enable_hangcheck,
"Periodically check GPU activity for detecting hangs. "
"WARNING: Disabling this can cause system wide hangs. "
"(default: true)");
 
-module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, 0400);
+module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, S_IRUSR);
 MODULE_PARM_DESC(enable_ppgtt,
"Override PPGTT usage. "
"(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with 
extended address space)");
 
-module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, 0400);
+module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, 
S_IRUSR);
 MODULE_PARM_DESC(enable_execlists,
"Override execlists usage. "
"(-1=auto [default], 0=disabled, 1=enabled)");