[PATCH 0/2] drm/i915: module paramater s

[PATCH 0/2] drm/i915: module paramater s

[Ben Widawsky]

The removal of the defunct parameter is second in case this is
considered an API breakage to anyone.

[PATCH 1/2] drm/i915: module parameter descriptions

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b54f7d9..7164650 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -39,39 +39,63 @@
 
 static int i915_modeset = -1;
 module_param_named(modeset, i915_modeset, int, 0400);
+MODULE_PARM_DESC(modeset, "Use kernel modesetting (KMS) "
+		 "(0=use DRM_I915_KMS from .config "
+		 "1=on "
+		 "-1=force vga console prio boost [default])");
 
 unsigned int i915_fbpercrtc = 0;
 module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400);
 
 int i915_panel_ignore_lid = 0;
 module_param_named(panel_ignore_lid, i915_panel_ignore_lid, int, 0600);
+MODULE_PARM_DESC(panel_ignore_lid, "Override lid status "
+		 "(0=autodetect [default], >0=lid open, <0=lid closed)");
 
 unsigned int i915_powersave = 1;
 module_param_named(powersave, i915_powersave, int, 0600);
+MODULE_PARM_DESC(powersave, "Powersaving features, fbc, downclocking, etc. "
+		 "(0=off, !0=on [default)");
 
 unsigned int i915_semaphores = 0;
 module_param_named(semaphores, i915_semaphores, int, 0600);
+MODULE_PARM_DESC(semaphores, "Use semaphores for inter-ring synchronization "
+		 "(0=off [default], !0=on)");
 
 unsigned int i915_enable_rc6 = 1;
 module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
+MODULE_PARM_DESC(i915_enable_rc6,
+		 "Enable render C-state 6 (0=off, !0=on [default])");
 
 unsigned int i915_enable_fbc = 0;
 module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
+MODULE_PARM_DESC(i915_enable_fbc,
+		 "Enable frame buffer compression for power savings "
+		 "(0=off [default], !0=on)");
 
 unsigned int i915_lvds_downclock = 0;
 module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
+MODULE_PARM_DESC(lvds_downclock,
+		 "LVDS downclocking for power savings(0=off [default], !0=on)");
 
 unsigned int i915_panel_use_ssc = 1;
 module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600);
+MODULE_PARM_DESC(lvds_use_ssc,
+		 "LVDS Spread Spectrum Clock (0=off, !0=on [default])");
 
 int i915_vbt_sdvo_panel_type = -1;
 module_param_named(vbt_sdvo_panel_type, i915_vbt_sdvo_panel_type, int, 0600);
+MODULE_PARM_DESC(vbt_sdvo_panel_type,
+		 "SDVO panel type override (Default: auto)");
 
 static bool i915_try_reset = true;
 module_param_named(reset, i915_try_reset, bool, 0600);
+MODULE_PARM_DESC(reset, "Attempt GPU resets (Default: true)");
 
 bool i915_enable_hangcheck = true;
 module_param_named(enable_hangcheck, i915_enable_hangcheck, bool, 0644);
+MODULE_PARM_DESC(enable_hangcheck,
+		 "Enable hanghceck triggered reset (Default: true)");
 
 static struct drm_driver driver;
 extern int intel_agp_enabled;
-- 
1.7.6

[PATCH 2/2] drm/i915: remove defunct fbpercrtc param

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.c |    3 ---
 drivers/gpu/drm/i915/i915_drv.h |    1 -
 2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7164650..2dfa8ed 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -44,9 +44,6 @@ MODULE_PARM_DESC(modeset, "Use kernel modesetting (KMS) "
 		 "1=on "
 		 "-1=force vga console prio boost [default])");
 
-unsigned int i915_fbpercrtc = 0;
-module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400);
-
 int i915_panel_ignore_lid = 0;
 module_param_named(panel_ignore_lid, i915_panel_ignore_lid, int, 0600);
 MODULE_PARM_DESC(panel_ignore_lid, "Override lid status "
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e0f9ca3..0ff21bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -985,7 +985,6 @@ struct drm_i915_file_private {
 
 extern struct drm_ioctl_desc i915_ioctls[];
 extern int i915_max_ioctl;
-extern unsigned int i915_fbpercrtc;
 extern int i915_panel_ignore_lid;
 extern unsigned int i915_powersave;
 extern unsigned int i915_semaphores;
-- 
1.7.6

Re: [PATCH 1/2] drm/i915: module parameter descriptions

[Paul Menzel]

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

Am Samstag, den 02.07.2011, 16:42 -0700 schrieb Ben Widawsky:

Without a verb in the commit summary it is hard to guess what the change
is about.

	drm/i915: Add module parameter descriptions

> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |   24 ++++++++++++++++++++++++
>  1 files changed, 24 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b54f7d9..7164650 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -39,39 +39,63 @@
>  
>  static int i915_modeset = -1;
>  module_param_named(modeset, i915_modeset, int, 0400);
> +MODULE_PARM_DESC(modeset, "Use kernel modesetting (KMS) "
> +		 "(0=use DRM_I915_KMS from .config "
> +		 "1=on "
> +		 "-1=force vga console prio boost [default])");

What does prio mean?

>  unsigned int i915_fbpercrtc = 0;
>  module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400);

If you are at it can you add a description here too?

>  int i915_panel_ignore_lid = 0;
>  module_param_named(panel_ignore_lid, i915_panel_ignore_lid, int, 0600);
> +MODULE_PARM_DESC(panel_ignore_lid, "Override lid status "
> +		 "(0=autodetect [default], >0=lid open, <0=lid closed)");

Above you put each option in one line.

>  unsigned int i915_powersave = 1;
>  module_param_named(powersave, i915_powersave, int, 0600);
> +MODULE_PARM_DESC(powersave, "Powersaving features, fbc, downclocking, etc. "
> +		 "(0=off, !0=on [default)");
>  
>  unsigned int i915_semaphores = 0;
>  module_param_named(semaphores, i915_semaphores, int, 0600);
> +MODULE_PARM_DESC(semaphores, "Use semaphores for inter-ring synchronization "
> +		 "(0=off [default], !0=on)");
>  
>  unsigned int i915_enable_rc6 = 1;
>  module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
> +MODULE_PARM_DESC(i915_enable_rc6,
> +		 "Enable render C-state 6 (0=off, !0=on [default])");
>  
>  unsigned int i915_enable_fbc = 0;
>  module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
> +MODULE_PARM_DESC(i915_enable_fbc,
> +		 "Enable frame buffer compression for power savings "
> +		 "(0=off [default], !0=on)");

Does that depend on `powersave` somehow?

>  unsigned int i915_lvds_downclock = 0;
>  module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
> +MODULE_PARM_DESC(lvds_downclock,
> +		 "LVDS downclocking for power savings(0=off [default], !0=on)");

Missing space before »(« and a verb like use or enable is missing.

>  unsigned int i915_panel_use_ssc = 1;
>  module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600);
> +MODULE_PARM_DESC(lvds_use_ssc,
> +		 "LVDS Spread Spectrum Clock (0=off, !0=on [default])");

A verb is missing.

>  int i915_vbt_sdvo_panel_type = -1;
>  module_param_named(vbt_sdvo_panel_type, i915_vbt_sdvo_panel_type, int, 0600);
> +MODULE_PARM_DESC(vbt_sdvo_panel_type,
> +		 "SDVO panel type override (Default: auto)");

A verb is missing and what values are allowed.

>  static bool i915_try_reset = true;
>  module_param_named(reset, i915_try_reset, bool, 0600);
> +MODULE_PARM_DESC(reset, "Attempt GPU resets (Default: true)");
>  
>  bool i915_enable_hangcheck = true;
>  module_param_named(enable_hangcheck, i915_enable_hangcheck, bool, 0644);
> +MODULE_PARM_DESC(enable_hangcheck,
> +		 "Enable hanghceck triggered reset (Default: true)");

s/hanghceck/hangcheck/
 
>  static struct drm_driver driver;
>  extern int intel_agp_enabled;


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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 2/2] drm/i915: remove defunct fbpercrtc param

[Paul Menzel]

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

Am Samstag, den 02.07.2011, 16:42 -0700 schrieb Ben Widawsky:
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |    3 ---
>  drivers/gpu/drm/i915/i915_drv.h |    1 -
>  2 files changed, 0 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 7164650..2dfa8ed 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -44,9 +44,6 @@ MODULE_PARM_DESC(modeset, "Use kernel modesetting (KMS) "
>  		 "1=on "
>  		 "-1=force vga console prio boost [default])");
>  
> -unsigned int i915_fbpercrtc = 0;
> -module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400);
> -

Ok, now I see why you did not add a description in the first place. I
would have applied this patch first then.

>  int i915_panel_ignore_lid = 0;
>  module_param_named(panel_ignore_lid, i915_panel_ignore_lid, int, 0600);
>  MODULE_PARM_DESC(panel_ignore_lid, "Override lid status "
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e0f9ca3..0ff21bc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -985,7 +985,6 @@ struct drm_i915_file_private {
>  
>  extern struct drm_ioctl_desc i915_ioctls[];
>  extern int i915_max_ioctl;
> -extern unsigned int i915_fbpercrtc;
>  extern int i915_panel_ignore_lid;
>  extern unsigned int i915_powersave;
>  extern unsigned int i915_semaphores;


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 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/2] drm/i915: module parameter descriptions

[Chris Wilson]

User-facing documentation brings out the pedant...

On Sat,  2 Jul 2011 16:42:33 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |   24 ++++++++++++++++++++++++
>  1 files changed, 24 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b54f7d9..7164650 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -39,39 +39,63 @@
>  
>  static int i915_modeset = -1;
>  module_param_named(modeset, i915_modeset, int, 0400);
> +MODULE_PARM_DESC(modeset, "Use kernel modesetting (KMS) "
> +		 "(0=use DRM_I915_KMS from .config "
> +		 "1=on "
> +		 "-1=force vga console prio boost [default])");
Commas required between the options.
  
>  unsigned int i915_fbpercrtc = 0;
>  module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400);
>  
>  int i915_panel_ignore_lid = 0;
>  module_param_named(panel_ignore_lid, i915_panel_ignore_lid, int, 0600);
> +MODULE_PARM_DESC(panel_ignore_lid, "Override lid status "
> +		 "(0=autodetect [default], >0=lid open, <0=lid closed)");
0, -1, 1. Let's not overcomplicate the instructions.
  
>  unsigned int i915_powersave = 1;
>  module_param_named(powersave, i915_powersave, int, 0600);
> +MODULE_PARM_DESC(powersave, "Powersaving features, fbc, downclocking, etc. "
> +		 "(0=off, !0=on [default)");
0 or 1, as above and below. In fact, I prefer the true/false as you used
later. Be consistent! :-)
  
>  unsigned int i915_semaphores = 0;
>  module_param_named(semaphores, i915_semaphores, int, 0600);
> +MODULE_PARM_DESC(semaphores, "Use semaphores for inter-ring synchronization "
> +		 "(0=off [default], !0=on)");
>  
>  unsigned int i915_enable_rc6 = 1;
>  module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
> +MODULE_PARM_DESC(i915_enable_rc6,
> +		 "Enable render C-state 6 (0=off, !0=on [default])");
"Enable power-saving render C-state 6 (default: true)");

>  
>  unsigned int i915_enable_fbc = 0;
>  module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
> +MODULE_PARM_DESC(i915_enable_fbc,
> +		 "Enable frame buffer compression for power savings "
> +		 "(0=off [default], !0=on)");
>  
>  unsigned int i915_lvds_downclock = 0;
>  module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
> +MODULE_PARM_DESC(lvds_downclock,
> +		 "LVDS downclocking for power savings(0=off [default], !0=on)");
"Use panel (LVDS/eDP) downclocking for power savings(default: false)");
 
>  unsigned int i915_panel_use_ssc = 1;
>  module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600);
> +MODULE_PARM_DESC(lvds_use_ssc,
> +		 "LVDS Spread Spectrum Clock (0=off, !0=on [default])");
"Use Spread Spectrum Clock with panels (LVDS/eDP) (default: true)");
  
>  int i915_vbt_sdvo_panel_type = -1;
>  module_param_named(vbt_sdvo_panel_type, i915_vbt_sdvo_panel_type, int, 0600);
> +MODULE_PARM_DESC(vbt_sdvo_panel_type,
> +		 "SDVO panel type override (Default: auto)");
"Override selection of SDVO panel mode in the VBT (Default: auto)");
  
>  static bool i915_try_reset = true;
>  module_param_named(reset, i915_try_reset, bool, 0600);
> +MODULE_PARM_DESC(reset, "Attempt GPU resets (Default: true)");
>  
>  bool i915_enable_hangcheck = true;
>  module_param_named(enable_hangcheck, i915_enable_hangcheck, bool, 0644);
> +MODULE_PARM_DESC(enable_hangcheck,
> +		 "Enable hanghceck triggered reset (Default: true)");
"Periodically check GPU activity for detecting hangs [aka hangcheck]
(Default:true)"
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/2] drm/i915: module paramater s

[Chris Wilson]

On Sat,  2 Jul 2011 16:42:32 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> The removal of the defunct parameter is second in case this is
> considered an API breakage to anyone.

Module parameters are a nuisance since if you specify one incorrectly, or
if it is unknown, it prevents the module from being loaded. Gah.
So I think we can live with it for a while longer. Please do move it out
of the main block and earmark it as being unused.

Also can you mark the parameters as __read_mostly?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre