[Intel-gfx] [PATCH] drm/i915: Make DP fast link training a module parameter

[Jani Nikula]

On Fri, 20 Nov 2015, Mika Kahola <mika.kahola at intel.com> wrote:
> There is a bug report
>
> https://bugs.freedesktop.org/show_bug.cgi?id=91393
>
> indicating that there are panels that do not support
> link training starting with non-zero voltage swing and
> pre-emphasis values.
>
> This patch proposes to make this fast link training
> feature as one module parameter. To take more conservative
> approach this feature is disabled by default.

Please let's not add any more of these. If we can't make this work
automatically (we should still keep trying) I'd say we remove it.

Adding features behind module parameters means that most people will
never use them. Theoretically it doubles our testing efforts because we
would have to test with the feature on and off; in practise that won't
happen and only the default will be tested (because we already have so
many features behind parameters). The code bitrots and breaks even for
the platforms on which it did work originally. Then there are a minority
of people who will enable all possible knobs they read about on forums
and think will improve performance or power savings or something, and
report bugs when they fail.

IMO there's just too much effort for too little gain if we can't enable
this. I just wish we would have been able to block more of the module
parameters earlier...


BR,
Jani.


>
> Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h               |  1 +
>  drivers/gpu/drm/i915/i915_params.c            |  4 ++++
>  drivers/gpu/drm/i915/intel_dp_link_training.c | 11 ++++++++++-
>  3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a47e0f4..dc2d5a0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2669,6 +2669,7 @@ struct i915_params {
>  	bool verbose_state_checks;
>  	bool nuclear_pageflip;
>  	int edp_vswing;
> +	bool enable_dp_flt;
>  };
>  extern struct i915_params i915 __read_mostly;
>  
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 835d609..aea5d47 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -56,6 +56,7 @@ struct i915_params i915 __read_mostly = {
>  	.edp_vswing = 0,
>  	.enable_guc_submission = false,
>  	.guc_log_level = -1,
> +	.enable_dp_flt = false,
>  };
>  
>  module_param_named(modeset, i915.modeset, int, 0400);
> @@ -202,3 +203,6 @@ MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:false)")
>  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_unsafe(enable_dp_flt, i915.enable_dp_flt, bool, 0400);
> +MODULE_PARM_DESC(enable_dp_flt, "Enable DP fast link training (default:false)");
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index 8888793..f8b6d69 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -85,8 +85,17 @@ static bool
>  intel_dp_reset_link_train(struct intel_dp *intel_dp,
>  			uint8_t dp_train_pat)
>  {
> -	if (!intel_dp->train_set_valid)
> +	if (i915.enable_dp_flt) {
> +		DRM_DEBUG_KMS("DP flt enabled, train set valid: %s\n",
> +			      intel_dp->train_set_valid ? "true" : "false");
> +
> +		if (!intel_dp->train_set_valid)
> +			memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
> +	} else {
> +		DRM_DEBUG_KMS("DP flt disabled\n");
>  		memset(intel_dp->train_set, 0, sizeof(intel_dp->train_set));
> +	}
> +
>  	intel_dp_set_signal_levels(intel_dp);
>  	return intel_dp_set_link_train(intel_dp, dp_train_pat);
>  }

-- 
Jani Nikula, Intel Open Source Technology Center