Re: [PATCH 09/11] drm/i915: New module param to control the size of buffer used for storing GuC firmware logs

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 27 Jun 2016, akash.goel@intel.com wrote:
> From: Akash Goel <akash.goel@intel.com>
>
> On recieving the log buffer flush interrupt from GuC firmware, Driver
> stores the snapshot of the log buffer in a local buffer, from which
> Userspace can pull the logs. By default Driver store, up to, 4 snapshots
> of the log buffer in a local buffer (managed by relay).
> Added a new module (read only) param, 'guc_log_size', through which User
> can specify the number of snapshots of log buffer to be stored in local
> buffer. This can be used to ensure capturing of all boot time logs even
> with high verbosity level.
>
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_guc_submission.c | 3 +--
>  drivers/gpu/drm/i915/i915_params.c         | 5 +++++
>  drivers/gpu/drm/i915/i915_params.h         | 1 +
>  3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index fd26a9e..8c0fd83 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -999,8 +999,7 @@ static void guc_create_log_relay_file(struct intel_guc *guc)
>  
>  	/* Keep the size of sub buffers same as shared log buffer */
>  	subbuf_size = guc->log_obj->base.size;
> -	/* TODO: Decide based on the User's input */
> -	n_subbufs = 4;
> +	n_subbufs = i915.guc_log_size;
>  
>  	guc_log_relay_chan = relay_open("guc_log", log_dir,
>  			subbuf_size, n_subbufs, &relay_callbacks, dev);
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 8b13bfa..14ce0c4 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -57,6 +57,7 @@ struct i915_params i915 __read_mostly = {
>  	.enable_guc_loading = -1,
>  	.enable_guc_submission = -1,
>  	.guc_log_level = -1,
> +	.guc_log_size = 4,
>  	.enable_dp_mst = true,
>  	.inject_load_failure = 0,
>  	.enable_dpcd_backlight = false,
> @@ -214,6 +215,10 @@ module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
>  MODULE_PARM_DESC(guc_log_level,
>  	"GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
>  
> +module_param_named(guc_log_size, i915.guc_log_size, int, 0400);
> +MODULE_PARM_DESC(guc_log_size,
> +	"Number of sub buffers to store GuC firmware logs (default: 4)");
> +

I guess my battle against adding all sorts of module parameters all the
time is a futile and lost one. :(

Please at least make it clear what the unit of the size is. It's not
obvious to me, and I shouldn't have to look at the source for that.

BR,
Jani.


>  module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, 0600);
>  MODULE_PARM_DESC(enable_dp_mst,
>  	"Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)");
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 0ad020b..89fa832 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -48,6 +48,7 @@ struct i915_params {
>  	int enable_guc_loading;
>  	int enable_guc_submission;
>  	int guc_log_level;
> +	int guc_log_size;
>  	int use_mmio_flip;
>  	int mmio_debug;
>  	int edp_vswing;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx